From 161770af00b000d317f66ccc3bbc3b66b39e238e Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sat, 18 Jul 2026 13:01:29 -0400 Subject: [PATCH 1/9] feat(openab-agent): xAI subscription login via OAuth device-code vendor Adds SuperGrok / X Premium subscription sign-in to openab-agent: - auth.rs: XaiVendor descriptor (namespace xai-oauth) + a generic RFC 8628 device-code login driver shared by all DeviceCode-grant vendors, with pure, unit-tested parsing/classification helpers (https-only verification URIs, interval-0 fallback, slow_down with and without a replacement interval, expired/denied terminal errors) - llm.rs: XaiProvider speaking OpenAI-compatible Chat Completions at api.x.ai/v1 with the OAuth access token as Bearer; pure xai_chat_messages transcript converter (tool adjacency preserved) - main.rs: `openab-agent auth xai-device` subcommand - acp.rs: session provider wiring, model switch, and static model list (grok-4.5 / grok-4.3 / grok-build-0.1, matching Pi's trimmed list) - docs/native-agent.md: env table + xAI credentials section Client id defaults to the grok CLI public client (ecosystem convention; xAI has no public OAuth client registration) and is overridable via OPENAB_AGENT_XAI_CLIENT_ID. Refresh reuses the existing generic driver, which already keeps the prior refresh_token when the AS omits it and defaults a missing expires_in. Closes #1423 --- docs/native-agent.md | 21 +- openab-agent/src/acp.rs | 25 ++ openab-agent/src/auth.rs | 496 ++++++++++++++++++++++++++++++++++++++- openab-agent/src/llm.rs | 296 ++++++++++++++++++++++- openab-agent/src/main.rs | 8 + 5 files changed, 837 insertions(+), 9 deletions(-) diff --git a/docs/native-agent.md b/docs/native-agent.md index f396d5e39..683a7dd6a 100644 --- a/docs/native-agent.md +++ b/docs/native-agent.md @@ -54,10 +54,13 @@ tolerated for forward-compatibility. | `OPENAB_AGENT_MODEL` | — (required for Anthropic) | Anthropic model id, optionally `provider/`-qualified (e.g. `claude-opus-4-8`, `anthropic/claude-opus-4-8`). No hardcoded default — dateless 4.6+ IDs are fixed canonical IDs that retire each generation, so the agent fails loud if unset rather than pin a model that will eventually 404. Overrides `model` in [config.json](#configuration-file-configjson). | | `OPENAB_AGENT_OPENAI_MODEL` | `gpt-5.4-mini` | Model to use (must be supported by your ChatGPT plan — see [Supported Models](#supported-models-chatgpt-subscription)) | | `OPENAB_AGENT_OPENAI_BASE_URL` | `https://chatgpt.com/backend-api` | API base URL | -| `OPENAB_AGENT_PROVIDER` | auto-detect | Force provider (`anthropic`, `openai`, `codex`) | +| `OPENAB_AGENT_XAI_MODEL` | `grok-4.5` | xAI model to use (see [xAI credentials](#xai-credentials-supergrok--x-premium)) | +| `OPENAB_AGENT_XAI_BASE_URL` | `https://api.x.ai/v1` | xAI API base URL | +| `OPENAB_AGENT_PROVIDER` | auto-detect | Force provider (`anthropic`, `openai`, `codex`, `xai`, `grok`) | | `OPENAB_AGENT_MAX_TOKENS` | `8192` | Max output tokens. Overrides `max_tokens` in config.json. | | `OPENAB_AGENT_OAUTH_CLIENT_ID` | Pi's client | Custom Codex OAuth client ID | | `OPENAB_AGENT_ANTHROPIC_CLIENT_ID` | Claude Code's client | Custom Anthropic OAuth client ID | +| `OPENAB_AGENT_XAI_CLIENT_ID` | grok CLI's client | Custom xAI OAuth client ID | | `OPENAB_AGENT_MAX_TOOL_LOOPS` | `50` | Max tool-call iterations per prompt before the agent gives up | | `ANTHROPIC_API_KEY` | — | Anthropic API key. Highest-precedence Anthropic credential (see [Anthropic credentials](#anthropic-credentials)). | | `CLAUDE_CODE_OAUTH_TOKEN` | — | Pre-provisioned long-lived Claude Pro/Max subscription token (from `claude setup-token`). Fleet route — no interactive login, no `auth.json` write. | @@ -114,6 +117,22 @@ Three ways to authenticate Anthropic, resolved in this **precedence** (ADR §5.3 A higher-precedence source's own errors (e.g. a key set but no model) surface rather than silently falling through to a lower one. +### xAI credentials (SuperGrok / X Premium) + +Sign in with a SuperGrok or X Premium subscription via the RFC 8628 device-code +flow — no `XAI_API_KEY` to provision, and headless-friendly (approve on any +device; ideal for pods via `kubectl exec`): + +```bash +openab-agent auth xai-device +``` + +Prints a user code and an `https://auth.x.ai/...` verification link (prefilled +when the server provides one). Tokens are stored under the `xai-oauth` tenant in +`auth.json` and refreshed automatically. Select the provider with +`OPENAB_AGENT_PROVIDER=xai` or a `xai/`-prefixed model (e.g. +`OPENAB_AGENT_MODEL=xai/grok-4.5`); xAI is not part of auto-detection. + ### Adding an OAuth vendor Subscription-OAuth providers are declared as a single `OAuthVendor` descriptor diff --git a/openab-agent/src/acp.rs b/openab-agent/src/acp.rs index cf4ca551b..cd1bda6fa 100644 --- a/openab-agent/src/acp.rs +++ b/openab-agent/src/acp.rs @@ -335,6 +335,16 @@ impl AcpServer { Err(e) => return self.error_response(id, -32000, &e), } } + "xai" | "grok" => { + let res = match model_override { + Some(m) => crate::llm::XaiProvider::from_auth_store_with_model(m), + None => crate::llm::XaiProvider::from_auth_store(), + }; + match res { + Ok(p) => (Box::new(p), "xai"), + Err(e) => return self.error_response(id, -32000, &e), + } + } _ => { // Auto-detect: Anthropic (API key or OAuth) first, then codex. let anthropic_res = match model_override { @@ -462,6 +472,9 @@ impl AcpServer { if crate::auth::load_tokens().is_ok() { models.extend(Self::static_openai_models()); } + if crate::auth::load_tokens_for(crate::auth::XAI_NAMESPACE).is_ok() { + models.extend(Self::static_xai_models()); + } if models.is_empty() { models.push(ModelOption::new( "none", @@ -505,6 +518,16 @@ impl AcpServer { ] } + fn static_xai_models() -> Vec { + // Static list matching Pi's trimmed xAI built-in models (earendil-works/pi + // #6734); grok-4.5 is the default the provider falls back to. + vec![ + ModelOption::new("grok-4.5", "Grok 4.5", "xai"), + ModelOption::new("grok-4.3", "Grok 4.3", "xai"), + ModelOption::new("grok-build-0.1", "Grok Build 0.1", "xai"), + ] + } + async fn handle_session_prompt(&mut self, id: u64, params: &Value) -> Vec { let session_id = params .get("sessionId") @@ -603,6 +626,8 @@ impl AcpServer { AnthropicProvider::from_oauth_auto_with_model(value).map(|p| Box::new(p) as _) } "anthropic" => AnthropicProvider::auto_with_model(value).map(|p| Box::new(p) as _), + "xai" => crate::llm::XaiProvider::from_auth_store_with_model(value) + .map(|p| Box::new(p) as _), _ => crate::llm::OpenAiProvider::from_auth_store_with_model(value) .map(|p| Box::new(p) as _), }; diff --git a/openab-agent/src/auth.rs b/openab-agent/src/auth.rs index 965d5e4ac..839402b61 100644 --- a/openab-agent/src/auth.rs +++ b/openab-agent/src/auth.rs @@ -14,6 +14,8 @@ use std::time::{SystemTime, UNIX_EPOCH}; const CODEX_NAMESPACE: &str = "codex"; /// Namespace key for the Anthropic (Claude Pro/Max) OAuth credential. pub const ANTHROPIC_NAMESPACE: &str = "anthropic-oauth"; +/// Namespace key for the xAI (SuperGrok / X Premium) OAuth credential. +pub const XAI_NAMESPACE: &str = "xai-oauth"; const REFRESH_SKEW_SECONDS: u64 = 120; @@ -33,6 +35,15 @@ const ANTHROPIC_REDIRECT_PORT: u16 = 53692; const ANTHROPIC_SCOPE: &str = "org:create_api_key user:profile user:inference user:sessions:claude_code user:mcp_servers user:file_upload"; +// xAI OAuth (SuperGrok / X Premium subscription) — RFC 8628 device-code flow. +// The client_id is the official grok CLI public client (no secret; appears in +// xai-org/grok-build and is reused by OpenClaw, Hermes, litellm, Warp, Pi, …). +// xAI exposes no public OAuth client registration, so reusing the CLI client is +// the ecosystem convention; `referrer` marks the requester (Pi sends "pi"). +const XAI_DEVICE_CODE_URL: &str = "https://auth.x.ai/oauth2/device/code"; +const XAI_TOKEN_URL: &str = "https://auth.x.ai/oauth2/token"; +const XAI_SCOPE: &str = "openid profile email offline_access grok-cli:access api:access"; + // ── OAuthVendor (auth axis — ADR §5.1) ────────────────────────────────────── // // A subscription-OAuth provider is one static `OAuthVendor` descriptor; the @@ -59,9 +70,10 @@ enum TokenBodyFormat { } /// OAuth grant a vendor's *primary* login uses. Codex additionally exposes a -/// device-code subcommand, but its browser login — like Anthropic's — is PKCE. +/// device-code subcommand (non-standard `device_auth_id` protocol, kept as a +/// bespoke flow), but its browser login — like Anthropic's — is PKCE. xAI is +/// the first device-primary vendor (RFC 8628 via `login_device_code_flow`). #[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[allow(dead_code)] // `DeviceCode` lands with the first device-primary vendor (copilot/kiro). enum AuthGrant { Pkce, DeviceCode, @@ -96,12 +108,22 @@ trait OAuthVendor: Send + Sync { fn token_body(&self) -> TokenBodyFormat { TokenBodyFormat::Form } - /// ADR §5.1 surface — `DeviceCode` lands with the first device-primary vendor - /// (copilot/kiro); both current vendors log in via PKCE, so unused until then. - #[allow(dead_code)] + /// Which grant the vendor's primary login uses. `login_device_code_flow` + /// guards on `DeviceCode` the same way `login_pkce_flow` guards on + /// `redirect()`, so a mis-wired vendor fails loud instead of half-flowing. fn grant(&self) -> AuthGrant { AuthGrant::Pkce } + /// RFC 8628 device authorization endpoint. `Some` for `DeviceCode`-grant + /// vendors; `None` for PKCE vendors (no device endpoint). + fn device_authorization_url(&self) -> Option<&str> { + None + } + /// Extra form fields on the device authorization request (xAI's `referrer` + /// requester tag — the device-flow analogue of `extra_authorize_params`). + fn extra_device_params(&self) -> &'static [(&'static str, &'static str)] { + &[] + } /// Full loopback redirect URI, derived from `redirect()`. fn redirect_uri(&self) -> Option { self.redirect() @@ -168,12 +190,48 @@ impl OAuthVendor for AnthropicVendor { } } +/// xAI (SuperGrok / X Premium subscription) — device-code-primary vendor. +/// Access tokens act as plain Bearer API keys against `api.x.ai/v1`. +struct XaiVendor; +impl OAuthVendor for XaiVendor { + fn namespace(&self) -> &str { + XAI_NAMESPACE + } + fn client_id(&self) -> String { + std::env::var("OPENAB_AGENT_XAI_CLIENT_ID") + .unwrap_or_else(|_| "b1a00492-073a-47ea-816f-4c329264a828".to_string()) + } + /// Device-primary vendor: no PKCE authorize endpoint exists. `redirect()` + /// is `None`, so `build_authorize_url` rejects this vendor before ever + /// reading this value; the device endpoint is returned as the least-wrong + /// stand-in for the mandatory trait method. + fn authorize_url(&self) -> &str { + XAI_DEVICE_CODE_URL + } + fn token_url(&self) -> &str { + XAI_TOKEN_URL + } + fn scope(&self) -> &str { + XAI_SCOPE + } + fn grant(&self) -> AuthGrant { + AuthGrant::DeviceCode + } + fn device_authorization_url(&self) -> Option<&str> { + Some(XAI_DEVICE_CODE_URL) + } + fn extra_device_params(&self) -> &'static [(&'static str, &'static str)] { + &[("referrer", "openab")] + } +} + /// Resolve a vendor descriptor by `auth.json` namespace. `None` for non-OAuth /// tenants (e.g. `mcp:`, whose refresh rmcp owns). fn vendor_for(namespace: &str) -> Option> { match namespace { CODEX_NAMESPACE => Some(Box::new(CodexVendor)), ANTHROPIC_NAMESPACE => Some(Box::new(AnthropicVendor)), + XAI_NAMESPACE => Some(Box::new(XaiVendor)), _ => None, } } @@ -477,6 +535,8 @@ fn write_auth_file(path: &Path, map: &HashMap) -> Result<()> fn auth_subcommand(namespace: &str) -> &'static str { if namespace == ANTHROPIC_NAMESPACE { "openab-agent auth anthropic-oauth" + } else if namespace == XAI_NAMESPACE { + "openab-agent auth xai-device" } else { "openab-agent auth codex-oauth" } @@ -1220,6 +1280,217 @@ pub async fn login_codex_device_flow() -> Result<()> { } } +// ── RFC 8628 device-code login driver (ADR §5.1, device axis) ────────────── +// +// Standards-compliant counterpart of `login_pkce_flow`: every `DeviceCode`-grant +// vendor reuses this one flow, parameterised by the descriptor. (The codex +// device flow above stays bespoke — OpenAI's `device_auth_id` protocol is not +// RFC 8628.) Parsing/classification are pure functions so the edge cases that +// bit other implementations (interval 0, absent `expires_in`, `slow_down` +// without a new interval, non-https verification URIs) are unit-tested without +// a live authorization server. + +/// Default poll interval when the AS omits `interval` or sends a non-positive +/// value (RFC 8628 §3.2 default). +const DEVICE_DEFAULT_INTERVAL_SECS: u64 = 5; +/// Deadline fallback when the AS omits `expires_in` (defensive; RFC 8628 +/// requires it, but a lenient default beats hard-failing an otherwise valid +/// grant). +const DEVICE_DEFAULT_EXPIRES_IN_SECS: u64 = 900; + +/// Parsed RFC 8628 §3.2 device authorization response. +struct DeviceAuthorization { + device_code: String, + user_code: String, + verification_uri: String, + verification_uri_complete: Option, + interval_secs: u64, + expires_in_secs: u64, +} + +/// The verification URI is shown to (and possibly opened by) the user; force +/// https so a malicious/tampered response can't direct them to an arbitrary +/// scheme or plaintext endpoint. +fn validate_https_url(raw: &str) -> Result { + let parsed = url::Url::parse(raw) + .map_err(|_| anyhow!("Untrusted verification URI in device authorization response"))?; + if parsed.scheme() != "https" { + return Err(anyhow!( + "Untrusted verification URI (non-https) in device authorization response" + )); + } + Ok(parsed.to_string()) +} + +fn required_device_field<'a>(body: &'a serde_json::Value, field: &str) -> Result<&'a str> { + body[field] + .as_str() + .filter(|s| !s.is_empty()) + .ok_or_else(|| anyhow!("No {field} in device authorization response")) +} + +fn parse_device_authorization(body: &serde_json::Value) -> Result { + let verification_uri_complete = match body["verification_uri_complete"] + .as_str() + .filter(|s| !s.is_empty()) + { + Some(raw) => Some(validate_https_url(raw)?), + None => None, + }; + Ok(DeviceAuthorization { + device_code: required_device_field(body, "device_code")?.to_string(), + user_code: required_device_field(body, "user_code")?.to_string(), + verification_uri: validate_https_url(required_device_field(body, "verification_uri")?)?, + verification_uri_complete, + // RFC 8628 allows interval 0 (no minimum wait) and some ASes send + // strings/garbage — fall back to the default rather than failing. + interval_secs: body["interval"] + .as_u64() + .filter(|v| *v > 0) + .unwrap_or(DEVICE_DEFAULT_INTERVAL_SECS), + expires_in_secs: body["expires_in"] + .as_u64() + .filter(|v| *v > 0) + .unwrap_or(DEVICE_DEFAULT_EXPIRES_IN_SECS), + }) +} + +/// What a non-2xx token poll means (RFC 8628 §3.5). +#[derive(Debug, PartialEq, Eq)] +enum DevicePollDisposition { + /// User hasn't approved yet — keep polling. + Pending, + /// Poll slower; the AS may supply a replacement interval. + SlowDown(Option), + /// Terminal — stop polling and surface the message. + Fatal(String), +} + +fn classify_device_poll_error(payload: &serde_json::Value) -> DevicePollDisposition { + let code = payload["error"].as_str().unwrap_or_default(); + match code { + "authorization_pending" => DevicePollDisposition::Pending, + "slow_down" => { + DevicePollDisposition::SlowDown(payload["interval"].as_u64().filter(|v| *v > 0)) + } + // xAI has been observed emitting the non-standard `authorization_denied`. + "access_denied" | "authorization_denied" => { + DevicePollDisposition::Fatal("authorization was denied".to_string()) + } + "expired_token" => { + DevicePollDisposition::Fatal("device code expired before authorization".to_string()) + } + other => { + let desc = payload["error_description"].as_str().unwrap_or_default(); + DevicePollDisposition::Fatal(if desc.is_empty() { + format!("unexpected device-code error: {other}") + } else { + format!("unexpected device-code error: {other} — {desc}") + }) + } + } +} + +/// Shared RFC 8628 login: request a device code, show the user code + +/// verification link (preferring the prefilled `verification_uri_complete`), +/// poll the token endpoint until approval, persist under the vendor namespace. +async fn login_device_code_flow(vendor: &dyn OAuthVendor) -> Result<()> { + if vendor.grant() != AuthGrant::DeviceCode { + return Err(anyhow!( + "{} is not a device-code vendor", + vendor.namespace() + )); + } + let device_url = vendor.device_authorization_url().ok_or_else(|| { + anyhow!( + "{} has no device authorization endpoint", + vendor.namespace() + ) + })?; + let client_id = vendor.client_id(); + let client = reqwest::Client::new(); + + let mut fields: Vec<(&str, &str)> = + vec![("client_id", client_id.as_str()), ("scope", vendor.scope())]; + fields.extend_from_slice(vendor.extra_device_params()); + let resp = client.post(device_url).form(&fields).send().await?; + if !resp.status().is_success() { + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + return Err(anyhow!( + "Device authorization request failed (HTTP {status}): {body}" + )); + } + let payload: serde_json::Value = resp.json().await?; + let device = parse_device_authorization(&payload)?; + + println!( + " Go to: {}", + device + .verification_uri_complete + .as_deref() + .unwrap_or(&device.verification_uri) + ); + println!(" Enter code: {}\n", device.user_code); + println!("Waiting for authorization..."); + + let deadline = + tokio::time::Instant::now() + tokio::time::Duration::from_secs(device.expires_in_secs); + let mut poll_interval = device.interval_secs; + loop { + tokio::time::sleep(tokio::time::Duration::from_secs(poll_interval)).await; + if tokio::time::Instant::now() >= deadline { + return Err(anyhow!( + "Device flow timed out after {} seconds.", + device.expires_in_secs + )); + } + let resp = client + .post(vendor.token_url()) + .form(&[ + ("grant_type", "urn:ietf:params:oauth:grant-type:device_code"), + ("client_id", client_id.as_str()), + ("device_code", device.device_code.as_str()), + ]) + .send() + .await?; + let status = resp.status(); + // Errors may arrive as non-JSON (proxies, HTML error pages) — treat an + // unparseable body as an empty object so classification still runs. + let text = resp.text().await.unwrap_or_default(); + let payload: serde_json::Value = + serde_json::from_str(&text).unwrap_or_else(|_| serde_json::json!({})); + if status.is_success() { + let store = token_store_from_payload(&payload, vendor.token_url(), vendor.namespace())?; + save_tokens_for(&store)?; + println!( + "\n\u{2705} Login successful! Token saved to {:?}", + auth_path() + ); + return Ok(()); + } + match classify_device_poll_error(&payload) { + DevicePollDisposition::Pending => continue, + DevicePollDisposition::SlowDown(server_interval) => { + // RFC 8628 §3.5: bump by 5s unless the AS supplied a new interval. + poll_interval = server_interval.unwrap_or(poll_interval + 5); + } + DevicePollDisposition::Fatal(msg) => { + return Err(anyhow!( + "{} device authorization failed (HTTP {status}): {msg}", + vendor.namespace() + )); + } + } + } +} + +/// xAI (SuperGrok / X Premium) RFC 8628 device-code login. +pub async fn login_xai_device_flow() -> Result<()> { + println!("Starting xAI (SuperGrok / X Premium) device-code login...\n"); + login_device_code_flow(&XaiVendor).await +} + pub fn show_status() { let path = auth_path(); let tokens: Vec = read_auth_file(&path) @@ -1238,7 +1509,7 @@ pub fn show_status() { if tokens.is_empty() { println!( - "Not authenticated.\nRun: openab-agent auth codex-oauth | openab-agent auth anthropic-oauth" + "Not authenticated.\nRun: openab-agent auth codex-oauth | openab-agent auth anthropic-oauth | openab-agent auth xai-device" ); return; } @@ -1351,6 +1622,219 @@ mod tests { ); } + // ── XaiVendor descriptor + RFC 8628 device-flow helpers ─────────────── + + #[test] + fn xai_vendor_descriptor_pins_wire_contract() { + assert_eq!(XaiVendor.namespace(), XAI_NAMESPACE); + assert_eq!(XaiVendor.grant(), AuthGrant::DeviceCode); + // Device-primary: no loopback redirect, form-encoded token bodies. + assert!(XaiVendor.redirect().is_none()); + assert!(XaiVendor.redirect_uri().is_none()); + assert_eq!(XaiVendor.token_body(), TokenBodyFormat::Form); + assert_eq!( + XaiVendor.device_authorization_url(), + Some("https://auth.x.ai/oauth2/device/code") + ); + assert_eq!(XaiVendor.token_url(), "https://auth.x.ai/oauth2/token"); + // grok-cli:access + api:access are what make the token usable as an + // API key against api.x.ai; offline_access is what yields a refresh token. + for scope in ["offline_access", "grok-cli:access", "api:access"] { + assert!(XaiVendor.scope().contains(scope), "missing scope {scope}"); + } + assert_eq!(XaiVendor.extra_device_params(), &[("referrer", "openab")]); + } + + #[test] + fn xai_vendor_is_not_a_pkce_vendor() { + // The PKCE driver must reject a device-primary vendor loudly. + let err = build_authorize_url(&XaiVendor, "CH", "ST").unwrap_err(); + assert!(err.to_string().contains("no loopback redirect"), "{err}"); + } + + #[test] + fn test_xai_client_id_default() { + temp_env::with_var("OPENAB_AGENT_XAI_CLIENT_ID", None::<&str>, || { + assert_eq!( + XaiVendor.client_id(), + "b1a00492-073a-47ea-816f-4c329264a828" + ); + }); + } + + #[test] + fn test_xai_client_id_override() { + temp_env::with_var("OPENAB_AGENT_XAI_CLIENT_ID", Some("custom_xai"), || { + assert_eq!(XaiVendor.client_id(), "custom_xai"); + }); + } + + #[test] + fn vendor_for_resolves_xai() { + assert_eq!( + vendor_for(XAI_NAMESPACE).unwrap().namespace(), + XAI_NAMESPACE + ); + } + + #[test] + fn auth_subcommand_per_namespace() { + assert_eq!( + auth_subcommand(XAI_NAMESPACE), + "openab-agent auth xai-device" + ); + assert_eq!( + auth_subcommand(ANTHROPIC_NAMESPACE), + "openab-agent auth anthropic-oauth" + ); + assert_eq!( + auth_subcommand(CODEX_NAMESPACE), + "openab-agent auth codex-oauth" + ); + } + + #[test] + fn validate_https_url_accepts_https_only() { + assert!(validate_https_url("https://auth.x.ai/device").is_ok()); + // Non-https schemes and garbage are untrusted — the URI is shown to / + // opened by the user. + assert!(validate_https_url("http://auth.x.ai/device").is_err()); + assert!(validate_https_url("file:///etc/passwd").is_err()); + assert!(validate_https_url("javascript:alert(1)").is_err()); + assert!(validate_https_url("not a url").is_err()); + } + + #[test] + fn parse_device_authorization_full_response() { + let d = parse_device_authorization(&serde_json::json!({ + "device_code": "dev123", + "user_code": "ABCD-EFGH", + "verification_uri": "https://auth.x.ai/device", + "verification_uri_complete": "https://auth.x.ai/device?code=ABCD-EFGH", + "interval": 7, + "expires_in": 600, + })) + .unwrap(); + assert_eq!(d.device_code, "dev123"); + assert_eq!(d.user_code, "ABCD-EFGH"); + assert_eq!(d.verification_uri, "https://auth.x.ai/device"); + assert_eq!( + d.verification_uri_complete.as_deref(), + Some("https://auth.x.ai/device?code=ABCD-EFGH") + ); + assert_eq!(d.interval_secs, 7); + assert_eq!(d.expires_in_secs, 600); + } + + #[test] + fn parse_device_authorization_defaults_interval_and_expiry() { + // RFC 8628 allows interval 0; expires_in absent is tolerated with a + // lenient default rather than a hard failure. + let d = parse_device_authorization(&serde_json::json!({ + "device_code": "d", + "user_code": "u", + "verification_uri": "https://auth.x.ai/device", + "interval": 0, + })) + .unwrap(); + assert_eq!(d.interval_secs, DEVICE_DEFAULT_INTERVAL_SECS); + assert_eq!(d.expires_in_secs, DEVICE_DEFAULT_EXPIRES_IN_SECS); + assert!(d.verification_uri_complete.is_none()); + } + + #[test] + fn parse_device_authorization_rejects_missing_or_untrusted_fields() { + // Missing device_code. + assert!(parse_device_authorization(&serde_json::json!({ + "user_code": "u", + "verification_uri": "https://auth.x.ai/device", + })) + .is_err()); + // Non-https verification_uri. + assert!(parse_device_authorization(&serde_json::json!({ + "device_code": "d", + "user_code": "u", + "verification_uri": "http://auth.x.ai/device", + })) + .is_err()); + // Non-https verification_uri_complete poisons an otherwise valid response. + assert!(parse_device_authorization(&serde_json::json!({ + "device_code": "d", + "user_code": "u", + "verification_uri": "https://auth.x.ai/device", + "verification_uri_complete": "http://evil.example/device", + })) + .is_err()); + } + + #[test] + fn classify_device_poll_error_dispositions() { + use DevicePollDisposition::*; + assert_eq!( + classify_device_poll_error(&serde_json::json!({"error": "authorization_pending"})), + Pending + ); + assert_eq!( + classify_device_poll_error(&serde_json::json!({"error": "slow_down"})), + SlowDown(None) + ); + assert_eq!( + classify_device_poll_error(&serde_json::json!({"error": "slow_down", "interval": 10})), + SlowDown(Some(10)) + ); + // Non-positive replacement interval is ignored (caller falls back to +5). + assert_eq!( + classify_device_poll_error(&serde_json::json!({"error": "slow_down", "interval": 0})), + SlowDown(None) + ); + for denied in ["access_denied", "authorization_denied"] { + assert!(matches!( + classify_device_poll_error(&serde_json::json!({"error": denied})), + Fatal(m) if m.contains("denied") + )); + } + assert!(matches!( + classify_device_poll_error(&serde_json::json!({"error": "expired_token"})), + Fatal(m) if m.contains("expired") + )); + assert!(matches!( + classify_device_poll_error( + &serde_json::json!({"error": "kaboom", "error_description": "detail"}) + ), + Fatal(m) if m.contains("kaboom") && m.contains("detail") + )); + // Unparseable/empty error body is terminal, not an infinite poll. + assert!(matches!( + classify_device_poll_error(&serde_json::json!({})), + Fatal(_) + )); + } + + #[test] + fn token_store_from_payload_defaults_expires_in() { + // xAI may omit expires_in on token responses — default 3600, not a failure. + let before = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_secs(); + let store = token_store_from_payload( + &serde_json::json!({"access_token": "at", "refresh_token": "rt"}), + XAI_TOKEN_URL, + XAI_NAMESPACE, + ) + .unwrap(); + assert_eq!(store.provider, XAI_NAMESPACE); + assert!(store.expires_at >= before + 3600); + // A refresh_token-less login response is a hard error (offline_access + // should always yield one on the initial device grant). + assert!(token_store_from_payload( + &serde_json::json!({"access_token": "at"}), + XAI_TOKEN_URL, + XAI_NAMESPACE, + ) + .is_err()); + } + #[test] fn test_is_expired_future_token() { let now = SystemTime::now() diff --git a/openab-agent/src/llm.rs b/openab-agent/src/llm.rs index baec41ae3..2edfc1e61 100644 --- a/openab-agent/src/llm.rs +++ b/openab-agent/src/llm.rs @@ -102,7 +102,15 @@ impl std::ops::Deref for SharedLlmProvider { /// the whole string is the model id — so a HuggingFace-style `org/model` id /// (e.g. `meta-llama/Llama-3-8B`) for a custom/OpenAI-compatible endpoint stays /// intact instead of mis-parsing `org` as a provider. Extend as vendors land. -const KNOWN_PROVIDERS: &[&str] = &["anthropic", "anthropic-oauth", "claude", "openai", "codex"]; +const KNOWN_PROVIDERS: &[&str] = &[ + "anthropic", + "anthropic-oauth", + "claude", + "openai", + "codex", + "xai", + "grok", +]; /// A model reference, optionally provider-qualified. Accepts the canonical /// `provider/model_id` form (e.g. `anthropic/claude-sonnet-4-6`) as well as a @@ -163,6 +171,7 @@ pub fn select_provider(choice: &str) -> Result, String> { "anthropic" => Ok(Box::new(AnthropicProvider::auto()?)), "anthropic-oauth" | "claude" => Ok(Box::new(AnthropicProvider::from_oauth_auto()?)), "openai" | "codex" => Ok(Box::new(OpenAiProvider::from_auth_store()?)), + "xai" | "grok" => Ok(Box::new(XaiProvider::from_auth_store()?)), _ => match AnthropicProvider::auto() { Ok(p) => Ok(Box::new(p)), // F3 — don't let a *present-but-misconfigured* Anthropic credential @@ -179,7 +188,7 @@ pub fn select_provider(choice: &str) -> Result, String> { OpenAiProvider::from_auth_store() .map(|p| Box::new(p) as Box) .map_err(|codex_err| format!( - "No credentials: set ANTHROPIC_API_KEY / CLAUDE_CODE_OAUTH_TOKEN, or run `openab-agent auth anthropic-oauth` / `openab-agent auth codex-oauth`. ({codex_err})" + "No credentials: set ANTHROPIC_API_KEY / CLAUDE_CODE_OAUTH_TOKEN, or run `openab-agent auth anthropic-oauth` / `openab-agent auth codex-oauth` / `openab-agent auth xai-device`. ({codex_err})" )) } } @@ -829,6 +838,198 @@ impl LlmProvider for OpenAiProvider { } } +// === xAI Provider (SuperGrok / X Premium subscription via device OAuth) === +// +// xAI's API is OpenAI Chat Completions-compatible at `api.x.ai/v1`; the OAuth +// access token from the `xai-oauth` tenant acts as a plain Bearer API key +// (scope `api:access`), so no bespoke wire format is needed — requests go to +// `/chat/completions` and responses reuse `parse_openai_response`'s +// Chat Completions path. + +pub struct XaiProvider { + base_url: String, + model: String, + client: reqwest::Client, +} + +impl XaiProvider { + /// Create provider using the stored xAI OAuth token from + /// `~/.openab/agent/auth.json` (run `openab-agent auth xai-device` first). + pub fn from_auth_store() -> Result { + // Just verify tokens exist; the live token is fetched (and refreshed) + // per call, mirroring `OpenAiProvider`. + crate::auth::load_tokens_for(crate::auth::XAI_NAMESPACE).map_err(|e| e.to_string())?; + Ok(Self { + base_url: std::env::var("OPENAB_AGENT_XAI_BASE_URL") + .unwrap_or_else(|_| "https://api.x.ai/v1".to_string()), + model: ModelRef::parse( + &std::env::var("OPENAB_AGENT_XAI_MODEL") + .or_else(|_| std::env::var("OPENAB_AGENT_MODEL")) + .unwrap_or_else(|_| "grok-4.5".to_string()), + ) + .model, + client: reqwest::Client::new(), + }) + } + + /// Create provider with a specific model override. + pub fn from_auth_store_with_model(model: &str) -> Result { + let mut p = Self::from_auth_store()?; + p.model = ModelRef::parse(model).model; + Ok(p) + } +} + +/// Convert the internal transcript to Chat Completions `messages`. Pure so the +/// mapping (tool_use → assistant `tool_calls`, tool_result → `role: tool`) is +/// unit-testable. Tool results are emitted *before* any user text from the same +/// message: Chat Completions requires `tool` messages to directly follow the +/// assistant message carrying the corresponding `tool_calls`. +fn xai_chat_messages(system: &str, messages: &[Message]) -> Vec { + let mut out: Vec = vec![json!({"role": "system", "content": system})]; + for m in messages { + if m.role == "user" { + for b in &m.content { + if let ContentBlock::ToolResult { + tool_use_id, + content, + .. + } = b + { + out.push(json!({ + "role": "tool", + "tool_call_id": tool_use_id, + "content": content, + })); + } + } + let texts: Vec<&str> = m + .content + .iter() + .filter_map(|b| match b { + ContentBlock::Text { text } => Some(text.as_str()), + _ => None, + }) + .collect(); + if !texts.is_empty() { + out.push(json!({"role": "user", "content": texts.join("")})); + } + } else if m.role == "assistant" { + let mut text_parts: Vec<&str> = Vec::new(); + let mut tool_calls: Vec = Vec::new(); + for b in &m.content { + match b { + ContentBlock::Text { text } => text_parts.push(text.as_str()), + ContentBlock::ToolUse { id, name, input } => { + tool_calls.push(json!({ + "id": id, + "type": "function", + "function": {"name": name, "arguments": input.to_string()}, + })); + } + _ => {} + } + } + if text_parts.is_empty() && tool_calls.is_empty() { + continue; + } + let mut msg = json!({"role": "assistant"}); + msg["content"] = if text_parts.is_empty() { + Value::Null + } else { + Value::String(text_parts.join("")) + }; + if !tool_calls.is_empty() { + msg["tool_calls"] = json!(tool_calls); + } + out.push(msg); + } + } + out +} + +impl LlmProvider for XaiProvider { + fn model(&self) -> &str { + &self.model + } + + fn is_oauth(&self) -> bool { + true + } + + fn chat<'a>( + &'a self, + system: &'a str, + messages: &'a [Message], + tools: &'a [ToolDef], + ) -> Pin>> + Send + 'a>> { + Box::pin(async move { + let mut body = json!({ + "model": &self.model, + "messages": xai_chat_messages(system, messages), + "stream": false, + }); + if !tools.is_empty() { + let cc_tools: Vec = tools + .iter() + .map(|t| { + json!({ + "type": "function", + "function": { + "name": &t.name, + "description": &t.description, + "parameters": &t.input_schema, + } + }) + }) + .collect(); + body["tools"] = json!(cc_tools); + body["tool_choice"] = json!("auto"); + } + + let max_retries = 3u32; + for attempt in 0..=max_retries { + let token = crate::auth::get_valid_token_for(crate::auth::XAI_NAMESPACE).await?; + let resp = self + .client + .post(format!("{}/chat/completions", self.base_url)) + .header("Authorization", format!("Bearer {token}")) + .header("Content-Type", "application/json") + .json(&body) + .send() + .await + .map_err(|e| anyhow!("HTTP request failed: {e}"))?; + + let status = resp.status(); + if (status.as_u16() == 429 || status.as_u16() == 529) && attempt < max_retries { + let delay = std::time::Duration::from_millis(1000 * 2u64.pow(attempt)); + tokio::time::sleep(delay).await; + continue; + } + + // 401: token may have expired mid-request, force refresh and retry + if status.as_u16() == 401 && attempt < max_retries { + let _ = crate::auth::force_refresh_for(crate::auth::XAI_NAMESPACE).await; + continue; + } + + if !status.is_success() { + let text = resp.text().await.unwrap_or_default(); + return Err(anyhow!("xAI API error {status}: {text}")); + } + + let payload: Value = resp + .json() + .await + .map_err(|e| anyhow!("Failed to parse xAI response: {e}"))?; + // Chat Completions shape → parse_openai_response's fallback path. + return parse_openai_response(&payload); + } + Err(anyhow!("xAI API: max retries exceeded")) + }) + } +} + fn extract_account_id_from_jwt(token: &str) -> Option { let parts: Vec<&str> = token.split('.').collect(); if parts.len() != 3 { @@ -1236,4 +1437,95 @@ mod tests { let resp = json!({"choices": []}); assert!(parse_openai_response(&resp).is_err()); } + + #[test] + fn test_model_ref_parses_xai_and_grok_prefixes() { + let r = ModelRef::parse("xai/grok-4.5"); + assert_eq!(r.provider.as_deref(), Some("xai")); + assert_eq!(r.model, "grok-4.5"); + let r = ModelRef::parse("grok/grok-4.3"); + assert_eq!(r.provider.as_deref(), Some("grok")); + assert_eq!(r.model, "grok-4.3"); + // Bare grok model id: no provider split. + let r = ModelRef::parse("grok-4.5"); + assert_eq!(r.provider, None); + assert_eq!(r.model, "grok-4.5"); + } + + #[test] + fn test_xai_chat_messages_maps_transcript_to_chat_completions() { + let messages = vec![ + Message { + role: "user".to_string(), + content: vec![ContentBlock::Text { + text: "list files".to_string(), + }], + }, + Message { + role: "assistant".to_string(), + content: vec![ + ContentBlock::Text { + text: "Listing.".to_string(), + }, + ContentBlock::ToolUse { + id: "call_1".to_string(), + name: "bash".to_string(), + input: json!({"command": "ls"}), + }, + ], + }, + Message { + role: "user".to_string(), + content: vec![ + // Text alongside a tool result: the tool message must still + // directly follow the assistant tool_calls message. + ContentBlock::Text { + text: "thanks".to_string(), + }, + ContentBlock::ToolResult { + tool_use_id: "call_1".to_string(), + content: "a.txt".to_string(), + is_error: None, + }, + ], + }, + ]; + let out = xai_chat_messages("sys", &messages); + assert_eq!(out[0]["role"], "system"); + assert_eq!(out[0]["content"], "sys"); + assert_eq!(out[1]["role"], "user"); + assert_eq!(out[1]["content"], "list files"); + // Assistant text + tool_calls in one message, arguments stringified. + assert_eq!(out[2]["role"], "assistant"); + assert_eq!(out[2]["content"], "Listing."); + assert_eq!(out[2]["tool_calls"][0]["id"], "call_1"); + assert_eq!(out[2]["tool_calls"][0]["type"], "function"); + assert_eq!(out[2]["tool_calls"][0]["function"]["name"], "bash"); + assert_eq!( + out[2]["tool_calls"][0]["function"]["arguments"], + "{\"command\":\"ls\"}" + ); + // Tool result emitted before the trailing user text (adjacency rule). + assert_eq!(out[3]["role"], "tool"); + assert_eq!(out[3]["tool_call_id"], "call_1"); + assert_eq!(out[3]["content"], "a.txt"); + assert_eq!(out[4]["role"], "user"); + assert_eq!(out[4]["content"], "thanks"); + } + + #[test] + fn test_xai_chat_messages_tool_only_assistant_has_null_content() { + let messages = vec![Message { + role: "assistant".to_string(), + content: vec![ContentBlock::ToolUse { + id: "call_2".to_string(), + name: "read".to_string(), + input: json!({"path": "x"}), + }], + }]; + let out = xai_chat_messages("s", &messages); + assert_eq!(out[1]["role"], "assistant"); + assert!(out[1]["content"].is_null()); + assert_eq!(out[1]["tool_calls"][0]["function"]["name"], "read"); + } } diff --git a/openab-agent/src/main.rs b/openab-agent/src/main.rs index 856045ae1..091ced816 100644 --- a/openab-agent/src/main.rs +++ b/openab-agent/src/main.rs @@ -93,6 +93,8 @@ enum AuthProvider { #[arg(long)] no_browser: bool, }, + /// xAI SuperGrok / X Premium via device code (RFC 8628, headless-friendly) + XaiDevice, /// Show stored credentials Status, } @@ -131,6 +133,12 @@ async fn main() { std::process::exit(1); } } + AuthProvider::XaiDevice => { + if let Err(e) = auth::login_xai_device_flow().await { + eprintln!("❌ Authentication failed: {e}"); + std::process::exit(1); + } + } AuthProvider::Status => { auth::show_status(); } From 340c76244520d667fd5d71fd6e8542781dd4dbee Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sat, 18 Jul 2026 13:28:52 -0400 Subject: [PATCH 2/9] =?UTF-8?q?fix(openab-agent):=20address=20review=20F1/?= =?UTF-8?q?F2=20=E2=80=94=20mixed=20text+tool=20turns=20and=20monotonic=20?= =?UTF-8?q?slow=5Fdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: the agent loop ended a turn whenever it carried text, even when tool_calls were also present — recorded but never executed (silently ends agentic turns on Chat Completions, where commentary before a call is common). Now the loop finishes only when a turn has no tool calls; accompanying text stays in the assistant message. Regression test uses a fail-fast unknown tool so it runs as a plain unit test. F2: a slow_down replacement interval below the current delay could speed polling up. RFC 8628 §3.5 requires increasing by 5s; the server-supplied interval is now honored only when it slows polling further (next_slow_down_interval, unit-tested). --- openab-agent/src/agent.rs | 54 +++++++++++++++++++++++++++++++++++++-- openab-agent/src/auth.rs | 23 +++++++++++++++-- 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/openab-agent/src/agent.rs b/openab-agent/src/agent.rs index cc7c5fb53..26b3715ec 100644 --- a/openab-agent/src/agent.rs +++ b/openab-agent/src/agent.rs @@ -232,8 +232,11 @@ impl Agent { content: assistant_content, }); - if tool_calls.is_empty() || !text_parts.is_empty() { - // No tool calls — we're done + // Done only when the turn carries no tool calls. A turn with BOTH + // text and tool_calls (common on Chat Completions — commentary + // before the call) must keep looping so the tools actually run; + // the text is already preserved in the assistant message above. + if tool_calls.is_empty() { final_text = text_parts.join(""); break; } @@ -372,6 +375,53 @@ mod tests { assert_eq!(result, "Hello!"); } + #[tokio::test] + async fn test_agent_mixed_text_and_tool_call_still_executes_tools() { + // Review F1: a turn carrying BOTH commentary text and tool_calls (common + // on Chat Completions) must keep looping so the tools actually run — not + // end the turn with the commentary while the calls sit unexecuted in + // history. The unknown tool name fails fast without touching the fs, so + // this exercises the full round-trip as a unit test. + let mock = MockLlmProvider::new(vec![ + vec![ + LlmEvent::Text("Let me check.".to_string()), + LlmEvent::ToolUse { + id: "tu_1".to_string(), + name: "no_such_tool".to_string(), + input: serde_json::json!({}), + }, + ], + vec![LlmEvent::Text("Done.".to_string()), LlmEvent::Stop], + ]); + + let tmp = tempfile::TempDir::new().unwrap(); + let mut agent = Agent::new(mock, tmp.path().to_string_lossy().to_string()); + let result = agent.run("go").await.unwrap(); + // The second LLM turn's text is the final reply — proof the loop continued. + assert_eq!(result, "Done."); + + // user, assistant(text+tool_use), user(tool_result), assistant(text) + assert_eq!(agent.messages.len(), 4); + match &agent.messages[1].content[..] { + [ContentBlock::Text { text }, ContentBlock::ToolUse { name, .. }] => { + assert_eq!(text, "Let me check."); + assert_eq!(name, "no_such_tool"); + } + other => panic!("unexpected assistant content: {other:?}"), + } + match &agent.messages[2].content[0] { + ContentBlock::ToolResult { + tool_use_id, + is_error, + .. + } => { + assert_eq!(tool_use_id, "tu_1"); + assert_eq!(*is_error, Some(true)); + } + other => panic!("expected ToolResult, got {other:?}"), + } + } + #[tokio::test] #[ignore] // Integration test: executes real file tools async fn test_agent_tool_call_then_response() { diff --git a/openab-agent/src/auth.rs b/openab-agent/src/auth.rs index 839402b61..d6f7ab8cf 100644 --- a/openab-agent/src/auth.rs +++ b/openab-agent/src/auth.rs @@ -1391,6 +1391,13 @@ fn classify_device_poll_error(payload: &serde_json::Value) -> DevicePollDisposit } } +/// RFC 8628 §3.5: after `slow_down` the client MUST increase its poll interval +/// by 5 seconds. A server-supplied replacement interval is honored only when it +/// slows polling down further — never to poll faster than before. +fn next_slow_down_interval(current: u64, server: Option) -> u64 { + (current + 5).max(server.unwrap_or(0)) +} + /// Shared RFC 8628 login: request a device code, show the user code + /// verification link (preferring the prefilled `verification_uri_complete`), /// poll the token endpoint until approval, persist under the vendor namespace. @@ -1472,8 +1479,7 @@ async fn login_device_code_flow(vendor: &dyn OAuthVendor) -> Result<()> { match classify_device_poll_error(&payload) { DevicePollDisposition::Pending => continue, DevicePollDisposition::SlowDown(server_interval) => { - // RFC 8628 §3.5: bump by 5s unless the AS supplied a new interval. - poll_interval = server_interval.unwrap_or(poll_interval + 5); + poll_interval = next_slow_down_interval(poll_interval, server_interval); } DevicePollDisposition::Fatal(msg) => { return Err(anyhow!( @@ -1810,6 +1816,19 @@ mod tests { )); } + #[test] + fn next_slow_down_interval_is_monotonic() { + // RFC 8628 §3.5 (review F2): slow_down must never speed polling up. + // No replacement interval → +5. + assert_eq!(next_slow_down_interval(5, None), 10); + // Replacement below the current delay is ignored in favor of +5. + assert_eq!(next_slow_down_interval(10, Some(3)), 15); + // Replacement equal to the bumped value is a no-op either way. + assert_eq!(next_slow_down_interval(10, Some(15)), 15); + // A genuinely slower server interval is honored. + assert_eq!(next_slow_down_interval(5, Some(30)), 30); + } + #[test] fn token_store_from_payload_defaults_expires_in() { // xAI may omit expires_in on token responses — default 3600, not a failure. From 87ad8b859621356eb80c22785ec0c02bf83fdcbc Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:00:02 -0400 Subject: [PATCH 3/9] fix(openab-agent): address review round-2 F1-F4 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: XaiProvider now resolves its model through xai_model() — OPENAB_AGENT_XAI_MODEL → OPENAB_AGENT_MODEL → config.json model → grok-4.5 — so a config-selected xai/grok-4.3 is no longer silently replaced by the default (env-over-config, ADR §5.5). F2: auth-mode preservation on model switch is now provider-specific. LlmProvider gains provider_name(); the ACP switch path preserves Anthropic OAuth only when the current session is Anthropic OAuth, so an xAI OAuth session switching to Anthropic honors ANTHROPIC_API_KEY. F3: the xAI 401 path refreshes at most once and only continues on a successful refresh; a failed refresh propagates its actionable re-login error. Deterministic tests cover refresh success (rotated Bearer on retry) and failure (invalid_grant surfaces) via canned local HTTP servers — no live xAI. F4: device token polling treats connection timeouts/refusals as transient per RFC 8628 §3.5 — exponential backoff (clamped 5..60s) and retry until the device-code deadline; other transport failures stay fatal. --- openab-agent/src/acp.rs | 80 ++++++++++- openab-agent/src/agent.rs | 7 + openab-agent/src/auth.rs | 47 ++++++- openab-agent/src/llm.rs | 274 ++++++++++++++++++++++++++++++++++++-- 4 files changed, 393 insertions(+), 15 deletions(-) diff --git a/openab-agent/src/acp.rs b/openab-agent/src/acp.rs index cd1bda6fa..76d9aec70 100644 --- a/openab-agent/src/acp.rs +++ b/openab-agent/src/acp.rs @@ -617,12 +617,20 @@ impl AcpServer { // Rebuild the current session's provider so the switch takes effect immediately if !session_id.is_empty() && self.sessions.contains_key(session_id) { - // Preserve the session's auth mode: an OAuth-forced session must not - // silently fall back to ANTHROPIC_API_KEY (which `auto_*` prefers). - let session_is_oauth = self.sessions[session_id].provider_is_oauth(); + // Preserve the session's auth mode per provider: an Anthropic + // OAuth-forced session must not silently fall back to + // ANTHROPIC_API_KEY (which `auto_*` prefers). Only an *Anthropic* + // OAuth session preserves that mode — a session on another OAuth + // provider (xAI, Codex) switching to Anthropic must still use + // `auto_with_model`, or it would bypass a configured API key and + // fail on deployments without an Anthropic OAuth tenant (F2). + let session_is_anthropic_oauth = { + let a = &self.sessions[session_id]; + a.provider_is_oauth() && a.provider_name() == "anthropic" + }; let new_provider: Result, String> = match provider_name { - "anthropic" if session_is_oauth => { + "anthropic" if session_is_anthropic_oauth => { AnthropicProvider::from_oauth_auto_with_model(value).map(|p| Box::new(p) as _) } "anthropic" => AnthropicProvider::auto_with_model(value).map(|p| Box::new(p) as _), @@ -1012,6 +1020,70 @@ mod tests { ); } + /// Minimal always-OAuth xAI-flavored provider so the F2 regression test + /// doesn't need a stored xai-oauth tenant on disk. + struct FakeXaiOauthProvider; + impl crate::llm::LlmProvider for FakeXaiOauthProvider { + fn model(&self) -> &str { + "grok-4.5" + } + fn is_oauth(&self) -> bool { + true + } + fn provider_name(&self) -> &str { + "xai" + } + fn chat<'a>( + &'a self, + _system: &'a str, + _messages: &'a [crate::llm::Message], + _tools: &'a [crate::llm::ToolDef], + ) -> std::pin::Pin< + Box< + dyn std::future::Future>> + + Send + + 'a, + >, + > { + Box::pin(async { Ok(vec![]) }) + } + } + + #[test] + fn test_switch_from_xai_oauth_to_anthropic_uses_api_key() { + // Review F2: OAuth-mode preservation must be provider-specific. A + // session on xAI OAuth switching to an Anthropic model must rebuild + // via `auto_with_model` (honoring ANTHROPIC_API_KEY) — NOT + // `from_oauth_auto_with_model`, which requires an Anthropic OAuth + // tenant and bypasses a configured API key. + let _guard = ENV_LOCK.lock().unwrap(); + let mut server = AcpServer::new(); + unsafe { std::env::set_var("ANTHROPIC_API_KEY", "test-key") }; + + let agent = Agent::new_boxed(Box::new(FakeXaiOauthProvider), "/tmp".to_string(), None); + server.sessions.insert("xai-session".to_string(), agent); + server.model_options = vec![ModelOption::new( + "claude-opus-4-20250514", + "Claude Opus 4", + "anthropic", + )]; + + let resp_str = server.handle_set_config_option( + 21, + &json!({ + "configId": "model", + "value": "claude-opus-4-20250514", + "sessionId": "xai-session", + }), + ); + unsafe { std::env::remove_var("ANTHROPIC_API_KEY") }; + let resp: Value = serde_json::from_str(&resp_str).unwrap(); + assert!( + resp["error"].is_null(), + "xAI OAuth → Anthropic switch must use the API key (F2): {resp}" + ); + } + #[tokio::test] async fn test_session_load_returns_config_options() { let _guard = ENV_LOCK.lock().unwrap(); diff --git a/openab-agent/src/agent.rs b/openab-agent/src/agent.rs index 26b3715ec..ff7958c60 100644 --- a/openab-agent/src/agent.rs +++ b/openab-agent/src/agent.rs @@ -113,6 +113,13 @@ impl Agent { self.provider.is_oauth() } + /// Canonical family name of the current provider (`anthropic` / `openai` / + /// `xai`). Combined with `provider_is_oauth` on model switch so auth-mode + /// preservation is provider-specific (review F2). + pub fn provider_name(&self) -> String { + self.provider.provider_name().to_string() + } + /// The model id the current provider will use. Authoritative source for the /// session's reported model (avoids a separate hardcoded default). pub fn provider_model(&self) -> String { diff --git a/openab-agent/src/auth.rs b/openab-agent/src/auth.rs index d6f7ab8cf..06c7cd724 100644 --- a/openab-agent/src/auth.rs +++ b/openab-agent/src/auth.rs @@ -1398,6 +1398,19 @@ fn next_slow_down_interval(current: u64, server: Option) -> u64 { (current + 5).max(server.unwrap_or(0)) } +/// Poll-interval ceiling for transport backoff, so a still-valid device code +/// keeps getting polled at a useful rate while the network flaps. +const DEVICE_MAX_POLL_INTERVAL_SECS: u64 = 60; + +/// RFC 8628 §3.5: on a connection timeout the client MUST reduce its polling +/// frequency before retrying, with exponential backoff recommended. Doubles +/// the interval, clamped to `[DEVICE_DEFAULT_INTERVAL_SECS, DEVICE_MAX_POLL_INTERVAL_SECS]`. +fn next_transport_backoff_interval(current: u64) -> u64 { + current + .saturating_mul(2) + .clamp(DEVICE_DEFAULT_INTERVAL_SECS, DEVICE_MAX_POLL_INTERVAL_SECS) +} + /// Shared RFC 8628 login: request a device code, show the user code + /// verification link (preferring the prefilled `verification_uri_complete`), /// poll the token endpoint until approval, persist under the vendor namespace. @@ -1452,7 +1465,7 @@ async fn login_device_code_flow(vendor: &dyn OAuthVendor) -> Result<()> { device.expires_in_secs )); } - let resp = client + let resp = match client .post(vendor.token_url()) .form(&[ ("grant_type", "urn:ietf:params:oauth:grant-type:device_code"), @@ -1460,7 +1473,25 @@ async fn login_device_code_flow(vendor: &dyn OAuthVendor) -> Result<()> { ("device_code", device.device_code.as_str()), ]) .send() - .await?; + .await + { + Ok(r) => r, + // RFC 8628 §3.5: a connection timeout is transient — the device + // code is still valid, so back off and keep polling until the + // deadline instead of destroying the in-progress login (F4). + Err(e) if e.is_timeout() || e.is_connect() => { + poll_interval = next_transport_backoff_interval(poll_interval); + tracing::warn!( + error = %e, + poll_interval, + "device token poll transport error; backing off and retrying" + ); + continue; + } + // Non-retryable transport failures (TLS, request build, redirect + // policy) stay explicit and fail the flow. + Err(e) => return Err(e.into()), + }; let status = resp.status(); // Errors may arrive as non-JSON (proxies, HTML error pages) — treat an // unparseable body as an empty object so classification still runs. @@ -1829,6 +1860,18 @@ mod tests { assert_eq!(next_slow_down_interval(5, Some(30)), 30); } + #[test] + fn next_transport_backoff_interval_doubles_and_clamps() { + // RFC 8628 §3.5 (review F4): reduce polling frequency after a + // connection timeout — exponential, clamped to a useful ceiling. + assert_eq!(next_transport_backoff_interval(5), 10); + assert_eq!(next_transport_backoff_interval(10), 20); + assert_eq!(next_transport_backoff_interval(40), 60); + assert_eq!(next_transport_backoff_interval(60), 60); + // Degenerate low values still land on the RFC default floor. + assert_eq!(next_transport_backoff_interval(0), 5); + } + #[test] fn token_store_from_payload_defaults_expires_in() { // xAI may omit expires_in on token responses — default 3600, not a failure. diff --git a/openab-agent/src/llm.rs b/openab-agent/src/llm.rs index 2edfc1e61..4096c29a7 100644 --- a/openab-agent/src/llm.rs +++ b/openab-agent/src/llm.rs @@ -75,6 +75,14 @@ pub trait LlmProvider: Send + Sync { fn is_oauth(&self) -> bool { false } + + /// Canonical provider family name (`anthropic` / `openai` / `xai`). + /// Combined with [`is_oauth`](Self::is_oauth) so a model switch preserves + /// auth mode *per provider*: an xAI OAuth session must not make a switch + /// to Anthropic bypass a configured `ANTHROPIC_API_KEY` (review F2). + fn provider_name(&self) -> &str { + "" + } } /// Shared, cloneable handle to an `LlmProvider`. A newtype over @@ -510,6 +518,10 @@ impl LlmProvider for AnthropicProvider { matches!(self.auth, AnthropicAuth::OAuth | AnthropicAuth::OAuthEnv(_)) } + fn provider_name(&self) -> &str { + "anthropic" + } + fn chat<'a>( &'a self, system: &'a str, @@ -686,6 +698,10 @@ impl LlmProvider for OpenAiProvider { &self.model } + fn provider_name(&self) -> &str { + "openai" + } + fn chat<'a>( &'a self, system: &'a str, @@ -852,6 +868,29 @@ pub struct XaiProvider { client: reqwest::Client, } +/// Resolve the xAI model. Precedence (env-over-config, ADR §5.5): +/// `OPENAB_AGENT_XAI_MODEL` → `OPENAB_AGENT_MODEL` → `model` in `config.json` +/// → built-in `grok-4.5`. Each source may be `provider/`-qualified +/// (`xai/grok-4.3`); the prefix is stripped via [`ModelRef`]. +fn xai_model() -> String { + if let Ok(m) = std::env::var("OPENAB_AGENT_XAI_MODEL") { + if !m.is_empty() { + return ModelRef::parse(&m).model; + } + } + if let Ok(m) = std::env::var("OPENAB_AGENT_MODEL") { + if !m.is_empty() { + return ModelRef::parse(&m).model; + } + } + if let Some(m) = crate::config::AgentConfig::load_or_default().model { + if !m.is_empty() { + return ModelRef::parse(&m).model; + } + } + "grok-4.5".to_string() +} + impl XaiProvider { /// Create provider using the stored xAI OAuth token from /// `~/.openab/agent/auth.json` (run `openab-agent auth xai-device` first). @@ -862,12 +901,7 @@ impl XaiProvider { Ok(Self { base_url: std::env::var("OPENAB_AGENT_XAI_BASE_URL") .unwrap_or_else(|_| "https://api.x.ai/v1".to_string()), - model: ModelRef::parse( - &std::env::var("OPENAB_AGENT_XAI_MODEL") - .or_else(|_| std::env::var("OPENAB_AGENT_MODEL")) - .unwrap_or_else(|_| "grok-4.5".to_string()), - ) - .model, + model: xai_model(), client: reqwest::Client::new(), }) } @@ -957,6 +991,10 @@ impl LlmProvider for XaiProvider { true } + fn provider_name(&self) -> &str { + "xai" + } + fn chat<'a>( &'a self, system: &'a str, @@ -988,6 +1026,7 @@ impl LlmProvider for XaiProvider { } let max_retries = 3u32; + let mut refreshed_after_401 = false; for attempt in 0..=max_retries { let token = crate::auth::get_valid_token_for(crate::auth::XAI_NAMESPACE).await?; let resp = self @@ -1007,9 +1046,16 @@ impl LlmProvider for XaiProvider { continue; } - // 401: token may have expired mid-request, force refresh and retry - if status.as_u16() == 401 && attempt < max_retries { - let _ = crate::auth::force_refresh_for(crate::auth::XAI_NAMESPACE).await; + // 401: the token may have expired mid-request. Reactive refresh + // at most once, and only continue on a *successful* refresh — a + // failed refresh (invalid_grant, storage error) must surface its + // actionable re-login message, not decay into a generic 401 + // after re-sending the same stale token (review F3). + if status.as_u16() == 401 && !refreshed_after_401 { + refreshed_after_401 = true; + crate::auth::force_refresh_for(crate::auth::XAI_NAMESPACE) + .await + .map_err(|e| anyhow!("xAI token refresh after HTTP 401 failed: {e}"))?; continue; } @@ -1528,4 +1574,214 @@ mod tests { assert!(out[1]["content"].is_null()); assert_eq!(out[1]["tool_calls"][0]["function"]["name"], "read"); } + + #[test] + fn xai_model_resolves_env_over_config_over_default() { + let dir = tempfile::tempdir().unwrap(); + let cfg = dir.path().join("config.json"); + std::fs::write(&cfg, r#"{"model":"xai/grok-4.3"}"#).unwrap(); + let cfg_path = cfg.to_str().unwrap(); + + // Config-only (review F1): the configured model must reach the + // provider, not be silently replaced by the built-in default. + temp_env::with_vars( + [ + ("OPENAB_CONFIG_PATH", Some(cfg_path)), + ("OPENAB_AGENT_XAI_MODEL", None), + ("OPENAB_AGENT_MODEL", None), + ("OPENAB_AGENT_PROVIDER", None), + ], + || { + assert_eq!(xai_model(), "grok-4.3"); + // Same config also selects the provider — the pair that F1 broke. + assert_eq!(resolve_provider_choice(), "xai"); + }, + ); + + // Env still wins over config. + temp_env::with_vars( + [ + ("OPENAB_CONFIG_PATH", Some(cfg_path)), + ("OPENAB_AGENT_XAI_MODEL", Some("xai/grok-4.5")), + ("OPENAB_AGENT_MODEL", None), + ], + || assert_eq!(xai_model(), "grok-4.5"), + ); + + // Nothing anywhere → built-in default. + let missing = dir.path().join("missing.json"); + temp_env::with_vars( + [ + ("OPENAB_CONFIG_PATH", Some(missing.to_str().unwrap())), + ("OPENAB_AGENT_XAI_MODEL", None), + ("OPENAB_AGENT_MODEL", None), + ], + || assert_eq!(xai_model(), "grok-4.5"), + ); + } + + // ── XaiProvider 401 reactive-refresh loop (review F3) ───────────────── + // Deterministic coverage via canned local HTTP servers: no live xAI, no + // real credentials. HOME is redirected to a tempdir so auth.json reads and + // the refresh POST stay inside the test sandbox (temp_env serialises + // env-mutating tests). + + fn http_resp(status: &str, body: &str) -> String { + format!( + "HTTP/1.1 {status}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ) + } + + /// Serve `responses` to sequential connections; returns the raw requests seen. + fn spawn_canned_http(responses: Vec) -> (String, std::thread::JoinHandle>) { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = listener.local_addr().unwrap(); + let handle = std::thread::spawn(move || { + let mut seen = Vec::new(); + for resp in responses { + let (mut stream, _) = listener.accept().unwrap(); + let mut buf = Vec::new(); + let mut tmp = [0u8; 1024]; + let mut header_end = None; + let mut content_len = 0usize; + loop { + let n = stream.read(&mut tmp).unwrap(); + if n == 0 { + break; + } + buf.extend_from_slice(&tmp[..n]); + if header_end.is_none() { + if let Some(pos) = buf.windows(4).position(|w| w == b"\r\n\r\n") { + header_end = Some(pos + 4); + let headers = String::from_utf8_lossy(&buf[..pos]); + content_len = headers + .lines() + .find_map(|l| { + let (k, v) = l.split_once(':')?; + if k.eq_ignore_ascii_case("content-length") { + v.trim().parse().ok() + } else { + None + } + }) + .unwrap_or(0); + } + } + if let Some(he) = header_end { + if buf.len() >= he + content_len { + break; + } + } + } + seen.push(String::from_utf8_lossy(&buf).to_string()); + stream.write_all(resp.as_bytes()).unwrap(); + } + seen + }); + (format!("http://{addr}"), handle) + } + + /// Write a temp-HOME auth.json holding one unexpired xai-oauth tenant whose + /// refresh endpoint points at `refresh_url`. + fn write_xai_auth(home: &std::path::Path, refresh_url: &str) { + let far_future = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs() + + 86_400; + let auth_dir = home.join(".openab").join("agent"); + std::fs::create_dir_all(&auth_dir).unwrap(); + std::fs::write( + auth_dir.join("auth.json"), + json!({ + "xai-oauth": { + "access_token": "stale-token", + "refresh_token": "rt1", + "expires_at": far_future, + "token_endpoint": refresh_url, + "provider": "xai-oauth", + } + }) + .to_string(), + ) + .unwrap(); + } + + #[test] + fn xai_chat_401_refresh_success_retries_with_fresh_token() { + let home = tempfile::tempdir().unwrap(); + // Chat endpoint: 401 first, then a successful completion. + let (chat_url, chat_handle) = spawn_canned_http(vec![ + http_resp("401 Unauthorized", r#"{"error":"unauthorized"}"#), + http_resp( + "200 OK", + r#"{"choices":[{"message":{"content":"ok"},"finish_reason":"stop"}]}"#, + ), + ]); + // Refresh endpoint: rotates the token successfully. + let (refresh_url, refresh_handle) = spawn_canned_http(vec![http_resp( + "200 OK", + r#"{"access_token":"fresh-token","refresh_token":"rt2","expires_in":3600}"#, + )]); + write_xai_auth(home.path(), &refresh_url); + + temp_env::with_var("HOME", Some(home.path().to_str().unwrap()), || { + let provider = XaiProvider { + base_url: chat_url.clone(), + model: "grok-4.5".to_string(), + client: reqwest::Client::new(), + }; + let events = tokio::runtime::Runtime::new() + .unwrap() + .block_on(provider.chat("sys", &[], &[])) + .unwrap(); + assert!(matches!(&events[0], LlmEvent::Text(t) if t == "ok")); + }); + + let chat_reqs = chat_handle.join().unwrap(); + assert!(chat_reqs[0].contains("Bearer stale-token")); + // The retry after a successful refresh must carry the rotated token. + assert!(chat_reqs[1].contains("Bearer fresh-token")); + let refresh_reqs = refresh_handle.join().unwrap(); + assert!(refresh_reqs[0].contains("grant_type=refresh_token")); + assert!(refresh_reqs[0].contains("rt1")); + } + + #[test] + fn xai_chat_401_refresh_failure_propagates_relogin_error() { + let home = tempfile::tempdir().unwrap(); + // Chat endpoint answers 401 once; a second request must never happen. + let (chat_url, _chat_handle) = spawn_canned_http(vec![http_resp( + "401 Unauthorized", + r#"{"error":"unauthorized"}"#, + )]); + // Refresh endpoint rejects the grant. + let (refresh_url, _refresh_handle) = spawn_canned_http(vec![http_resp( + "400 Bad Request", + r#"{"error":"invalid_grant"}"#, + )]); + write_xai_auth(home.path(), &refresh_url); + + temp_env::with_var("HOME", Some(home.path().to_str().unwrap()), || { + let provider = XaiProvider { + base_url: chat_url.clone(), + model: "grok-4.5".to_string(), + client: reqwest::Client::new(), + }; + let err = tokio::runtime::Runtime::new() + .unwrap() + .block_on(provider.chat("sys", &[], &[])) + .unwrap_err() + .to_string(); + // The actionable refresh failure surfaces (with the re-login hint + // from the shared refresh driver), not a generic xAI 401. + assert!( + err.contains("xAI token refresh after HTTP 401 failed"), + "got: {err}" + ); + assert!(err.contains("openab-agent auth xai-device"), "got: {err}"); + }); + } } From a8858597e18a850b23bf31d6a7b69d28790726fd Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:50:17 -0400 Subject: [PATCH 4/9] fix(openab-agent): address review round-3 F1-F3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit F1: OPENAB_AGENT_XAI_BASE_URL is validated before the OAuth bearer is attached — https only, x.ai hosts only (api.x.ai or *.x.ai). A typo'd, plaintext, or non-xAI proxy value now fails loud instead of leaking a refreshable subscription credential. Documented in native-agent.md. F2: the Anthropic OAuth preference is now sticky per session: it is recorded whenever an Anthropic provider is active and retained while other providers run, so Anthropic-OAuth → xAI → Anthropic returns to OAuth instead of silently switching to ANTHROPIC_API_KEY (different account/billing). Covered at agent level (deterministic round-trip) and acp level (switch-back takes the OAuth path). F3: xAI requests now carry the documented OPENAB_AGENT_MAX_TOKENS limit (env → config.json → 8192) via an extracted, unit-tested xai_request_body builder. --- docs/native-agent.md | 2 +- openab-agent/src/acp.rs | 92 +++++++++++++++++++++++--- openab-agent/src/agent.rs | 108 ++++++++++++++++++++++++++---- openab-agent/src/llm.rs | 136 +++++++++++++++++++++++++++++++------- 4 files changed, 290 insertions(+), 48 deletions(-) diff --git a/docs/native-agent.md b/docs/native-agent.md index 683a7dd6a..562e49042 100644 --- a/docs/native-agent.md +++ b/docs/native-agent.md @@ -55,7 +55,7 @@ tolerated for forward-compatibility. | `OPENAB_AGENT_OPENAI_MODEL` | `gpt-5.4-mini` | Model to use (must be supported by your ChatGPT plan — see [Supported Models](#supported-models-chatgpt-subscription)) | | `OPENAB_AGENT_OPENAI_BASE_URL` | `https://chatgpt.com/backend-api` | API base URL | | `OPENAB_AGENT_XAI_MODEL` | `grok-4.5` | xAI model to use (see [xAI credentials](#xai-credentials-supergrok--x-premium)) | -| `OPENAB_AGENT_XAI_BASE_URL` | `https://api.x.ai/v1` | xAI API base URL | +| `OPENAB_AGENT_XAI_BASE_URL` | `https://api.x.ai/v1` | xAI API base URL. Must be an `https://` URL on an `x.ai` host — the OAuth bearer is never sent elsewhere. | | `OPENAB_AGENT_PROVIDER` | auto-detect | Force provider (`anthropic`, `openai`, `codex`, `xai`, `grok`) | | `OPENAB_AGENT_MAX_TOKENS` | `8192` | Max output tokens. Overrides `max_tokens` in config.json. | | `OPENAB_AGENT_OAUTH_CLIENT_ID` | Pi's client | Custom Codex OAuth client ID | diff --git a/openab-agent/src/acp.rs b/openab-agent/src/acp.rs index 76d9aec70..ba2e943de 100644 --- a/openab-agent/src/acp.rs +++ b/openab-agent/src/acp.rs @@ -618,19 +618,17 @@ impl AcpServer { // Rebuild the current session's provider so the switch takes effect immediately if !session_id.is_empty() && self.sessions.contains_key(session_id) { // Preserve the session's auth mode per provider: an Anthropic - // OAuth-forced session must not silently fall back to - // ANTHROPIC_API_KEY (which `auto_*` prefers). Only an *Anthropic* - // OAuth session preserves that mode — a session on another OAuth - // provider (xAI, Codex) switching to Anthropic must still use - // `auto_with_model`, or it would bypass a configured API key and - // fail on deployments without an Anthropic OAuth tenant (F2). - let session_is_anthropic_oauth = { - let a = &self.sessions[session_id]; - a.provider_is_oauth() && a.provider_name() == "anthropic" - }; + // OAuth session must not silently fall back to ANTHROPIC_API_KEY + // (which `auto_*` prefers). The preference is *sticky* on the + // session (round-3 F2): an Anthropic-OAuth → xAI → Anthropic + // round trip still returns to OAuth, while a session that never + // chose Anthropic OAuth (e.g. created on xAI) rebuilds via + // `auto_with_model`, honoring a configured API key (round-2 F2). + let session_prefers_anthropic_oauth = + self.sessions[session_id].prefers_anthropic_oauth(); let new_provider: Result, String> = match provider_name { - "anthropic" if session_is_anthropic_oauth => { + "anthropic" if session_prefers_anthropic_oauth => { AnthropicProvider::from_oauth_auto_with_model(value).map(|p| Box::new(p) as _) } "anthropic" => AnthropicProvider::auto_with_model(value).map(|p| Box::new(p) as _), @@ -1049,6 +1047,78 @@ mod tests { } } + /// Anthropic-OAuth-flavored stub for the round-trip test below. + struct FakeAnthropicOauthProvider; + impl crate::llm::LlmProvider for FakeAnthropicOauthProvider { + fn model(&self) -> &str { + "claude-opus-4-20250514" + } + fn is_oauth(&self) -> bool { + true + } + fn provider_name(&self) -> &str { + "anthropic" + } + fn chat<'a>( + &'a self, + _system: &'a str, + _messages: &'a [crate::llm::Message], + _tools: &'a [crate::llm::ToolDef], + ) -> std::pin::Pin< + Box< + dyn std::future::Future>> + + Send + + 'a, + >, + > { + Box::pin(async { Ok(vec![]) }) + } + } + + #[test] + fn test_anthropic_oauth_round_trip_via_xai_keeps_oauth_mode() { + // Review round-3 F2: Anthropic OAuth → xAI → Anthropic (with + // ANTHROPIC_API_KEY set) must return to the OAuth path, not silently + // switch to the API key. With no anthropic-oauth tenant on disk the + // OAuth rebuild fails loudly — an error here proves the OAuth path was + // taken; a silent success would mean the API key hijacked the session + // (the regression this test pins). + let _guard = ENV_LOCK.lock().unwrap(); + let mut server = AcpServer::new(); + unsafe { std::env::set_var("ANTHROPIC_API_KEY", "test-key") }; + + // Session explicitly created on Anthropic OAuth, then moved to xAI. + let mut agent = Agent::new_boxed( + Box::new(FakeAnthropicOauthProvider), + "/tmp".to_string(), + None, + ); + agent.swap_provider(Box::new(FakeXaiOauthProvider)); + assert!(agent.prefers_anthropic_oauth(), "sticky policy lost"); + server.sessions.insert("rt-session".to_string(), agent); + server.model_options = vec![ModelOption::new( + "claude-opus-4-20250514", + "Claude Opus 4", + "anthropic", + )]; + + let resp_str = server.handle_set_config_option( + 22, + &json!({ + "configId": "model", + "value": "claude-opus-4-20250514", + "sessionId": "rt-session", + }), + ); + unsafe { std::env::remove_var("ANTHROPIC_API_KEY") }; + let resp: Value = serde_json::from_str(&resp_str).unwrap(); + assert!( + resp["error"].is_object(), + "round trip must take the OAuth path (fails without a tenant), \ + not silently rebuild on ANTHROPIC_API_KEY: {resp}" + ); + } + #[test] fn test_switch_from_xai_oauth_to_anthropic_uses_api_key() { // Review F2: OAuth-mode preservation must be provider-specific. A diff --git a/openab-agent/src/agent.rs b/openab-agent/src/agent.rs index ff7958c60..fe6f4929a 100644 --- a/openab-agent/src/agent.rs +++ b/openab-agent/src/agent.rs @@ -63,12 +63,29 @@ pub struct Agent { system_prompt: String, tools: Vec, mcp_manager: Option, + /// Sticky per-session auth policy for Anthropic (review round-3 F2): set + /// whenever an Anthropic provider is active, and *retained* while other + /// providers (xAI, Codex) are active, so an Anthropic-OAuth → xAI → + /// Anthropic round trip switches back to OAuth instead of silently + /// preferring `ANTHROPIC_API_KEY` (a different account/billing context). + anthropic_oauth_preferred: bool, +} + +/// The sticky-preference update shared by construction and provider swap: +/// only an *active Anthropic* provider rewrites the remembered policy. +fn anthropic_oauth_preference(provider: &dyn LlmProvider, previous: bool) -> bool { + if provider.provider_name() == "anthropic" { + provider.is_oauth() + } else { + previous + } } impl Agent { #[cfg(test)] pub fn new(provider: impl LlmProvider + 'static, working_dir: String) -> Self { let system_prompt = Self::build_system_prompt(&working_dir, None); + let anthropic_oauth_preferred = anthropic_oauth_preference(&provider, false); Self { provider: Box::new(provider), messages: Vec::new(), @@ -76,6 +93,7 @@ impl Agent { system_prompt, tools: tools::tool_definitions(), mcp_manager: None, + anthropic_oauth_preferred, } } @@ -92,6 +110,7 @@ impl Agent { } t }; + let anthropic_oauth_preferred = anthropic_oauth_preference(provider.as_ref(), false); Self { provider, messages: Vec::new(), @@ -99,25 +118,23 @@ impl Agent { system_prompt, tools, mcp_manager, + anthropic_oauth_preferred, } } - /// Replace the LLM provider while preserving conversation history. + /// Replace the LLM provider while preserving conversation history (and the + /// sticky Anthropic auth policy — see `anthropic_oauth_preferred`). pub fn swap_provider(&mut self, provider: Box) { + self.anthropic_oauth_preferred = + anthropic_oauth_preference(provider.as_ref(), self.anthropic_oauth_preferred); self.provider = provider; } - /// True if the current provider authenticates via OAuth. Used on model - /// switch to rebuild with the same auth mode. - pub fn provider_is_oauth(&self) -> bool { - self.provider.is_oauth() - } - - /// Canonical family name of the current provider (`anthropic` / `openai` / - /// `xai`). Combined with `provider_is_oauth` on model switch so auth-mode - /// preservation is provider-specific (review F2). - pub fn provider_name(&self) -> String { - self.provider.provider_name().to_string() + /// Sticky Anthropic auth policy for this session (review round-3 F2): + /// true when the session most recently ran Anthropic in OAuth mode, even + /// if another provider is active right now. + pub fn prefers_anthropic_oauth(&self) -> bool { + self.anthropic_oauth_preferred } /// The model id the current provider will use. Authoritative source for the @@ -382,6 +399,73 @@ mod tests { assert_eq!(result, "Hello!"); } + /// Stub provider with a fixed identity, for auth-policy tests. + struct StubProvider { + name: &'static str, + oauth: bool, + } + impl LlmProvider for StubProvider { + fn model(&self) -> &str { + "stub" + } + fn is_oauth(&self) -> bool { + self.oauth + } + fn provider_name(&self) -> &str { + self.name + } + fn chat<'a>( + &'a self, + _system: &'a str, + _messages: &'a [Message], + _tools: &'a [ToolDef], + ) -> std::pin::Pin>> + Send + 'a>> + { + Box::pin(async { Ok(vec![]) }) + } + } + + #[test] + fn anthropic_oauth_preference_is_sticky_across_provider_round_trips() { + // Review round-3 F2: Anthropic-OAuth → xAI → Anthropic must remember + // the OAuth policy; only an *active Anthropic* provider rewrites it. + let tmp = tempfile::TempDir::new().unwrap(); + let mut agent = Agent::new_boxed( + Box::new(StubProvider { + name: "anthropic", + oauth: true, + }), + tmp.path().to_string_lossy().to_string(), + None, + ); + assert!(agent.prefers_anthropic_oauth()); + + // Switching away to xAI keeps the remembered Anthropic policy. + agent.swap_provider(Box::new(StubProvider { + name: "xai", + oauth: true, + })); + assert!(agent.prefers_anthropic_oauth(), "policy lost on round trip"); + + // Explicitly running Anthropic on an API key rewrites the policy… + agent.swap_provider(Box::new(StubProvider { + name: "anthropic", + oauth: false, + })); + assert!(!agent.prefers_anthropic_oauth()); + + // …and a session that never chose Anthropic OAuth never prefers it. + let agent2 = Agent::new_boxed( + Box::new(StubProvider { + name: "xai", + oauth: true, + }), + tmp.path().to_string_lossy().to_string(), + None, + ); + assert!(!agent2.prefers_anthropic_oauth()); + } + #[tokio::test] async fn test_agent_mixed_text_and_tool_call_still_executes_tools() { // Review F1: a turn carrying BOTH commentary text and tool_calls (common diff --git a/openab-agent/src/llm.rs b/openab-agent/src/llm.rs index 4096c29a7..8467c43a2 100644 --- a/openab-agent/src/llm.rs +++ b/openab-agent/src/llm.rs @@ -865,6 +865,7 @@ impl LlmProvider for OpenAiProvider { pub struct XaiProvider { base_url: String, model: String, + max_tokens: u32, client: reqwest::Client, } @@ -891,6 +892,28 @@ fn xai_model() -> String { "grok-4.5".to_string() } +/// Validate `OPENAB_AGENT_XAI_BASE_URL` before the OAuth bearer is attached +/// (review round-3 F1): the stored subscription token is a refreshable +/// credential, so it may only ever be sent over https to an xAI-owned host +/// (`api.x.ai` or another `*.x.ai` subdomain). A typo'd, plaintext, or +/// non-xAI proxy value fails loud instead of silently leaking the token. +fn validate_xai_base_url(raw: &str) -> Result { + let parsed = url::Url::parse(raw) + .map_err(|e| format!("invalid OPENAB_AGENT_XAI_BASE_URL `{raw}`: {e}"))?; + if parsed.scheme() != "https" { + return Err(format!( + "refusing OPENAB_AGENT_XAI_BASE_URL `{raw}`: the xAI OAuth bearer may only be sent over https" + )); + } + let host = parsed.host_str().unwrap_or_default(); + if host != "x.ai" && !host.ends_with(".x.ai") { + return Err(format!( + "refusing OPENAB_AGENT_XAI_BASE_URL `{raw}`: the xAI OAuth bearer may only be sent to an x.ai host (got `{host}`)" + )); + } + Ok(raw.trim_end_matches('/').to_string()) +} + impl XaiProvider { /// Create provider using the stored xAI OAuth token from /// `~/.openab/agent/auth.json` (run `openab-agent auth xai-device` first). @@ -898,10 +921,16 @@ impl XaiProvider { // Just verify tokens exist; the live token is fetched (and refreshed) // per call, mirroring `OpenAiProvider`. crate::auth::load_tokens_for(crate::auth::XAI_NAMESPACE).map_err(|e| e.to_string())?; + let base_url = match std::env::var("OPENAB_AGENT_XAI_BASE_URL") { + Ok(raw) if !raw.is_empty() => validate_xai_base_url(&raw)?, + _ => "https://api.x.ai/v1".to_string(), + }; Ok(Self { - base_url: std::env::var("OPENAB_AGENT_XAI_BASE_URL") - .unwrap_or_else(|_| "https://api.x.ai/v1".to_string()), + base_url, model: xai_model(), + // Same documented env-over-config resolution as Anthropic: + // OPENAB_AGENT_MAX_TOKENS → config.json max_tokens → 8192. + max_tokens: anthropic_max_tokens(), client: reqwest::Client::new(), }) } @@ -982,6 +1011,42 @@ fn xai_chat_messages(system: &str, messages: &[Message]) -> Vec { out } +/// Build the Chat Completions request body. Pure so the wire shape — including +/// the documented `OPENAB_AGENT_MAX_TOKENS` output limit (review round-3 F3) — +/// is unit-testable. +fn xai_request_body( + model: &str, + max_tokens: u32, + system: &str, + messages: &[Message], + tools: &[ToolDef], +) -> Value { + let mut body = json!({ + "model": model, + "messages": xai_chat_messages(system, messages), + "max_tokens": max_tokens, + "stream": false, + }); + if !tools.is_empty() { + let cc_tools: Vec = tools + .iter() + .map(|t| { + json!({ + "type": "function", + "function": { + "name": &t.name, + "description": &t.description, + "parameters": &t.input_schema, + } + }) + }) + .collect(); + body["tools"] = json!(cc_tools); + body["tool_choice"] = json!("auto"); + } + body +} + impl LlmProvider for XaiProvider { fn model(&self) -> &str { &self.model @@ -1002,28 +1067,7 @@ impl LlmProvider for XaiProvider { tools: &'a [ToolDef], ) -> Pin>> + Send + 'a>> { Box::pin(async move { - let mut body = json!({ - "model": &self.model, - "messages": xai_chat_messages(system, messages), - "stream": false, - }); - if !tools.is_empty() { - let cc_tools: Vec = tools - .iter() - .map(|t| { - json!({ - "type": "function", - "function": { - "name": &t.name, - "description": &t.description, - "parameters": &t.input_schema, - } - }) - }) - .collect(); - body["tools"] = json!(cc_tools); - body["tool_choice"] = json!("auto"); - } + let body = xai_request_body(&self.model, self.max_tokens, system, messages, tools); let max_retries = 3u32; let mut refreshed_after_401 = false; @@ -1620,6 +1664,48 @@ mod tests { ); } + #[test] + fn validate_xai_base_url_allows_only_https_x_ai_hosts() { + // Review round-3 F1: the OAuth bearer must never leave the x.ai trust + // boundary or travel over plaintext. + assert_eq!( + validate_xai_base_url("https://api.x.ai/v1").unwrap(), + "https://api.x.ai/v1" + ); + // Trailing slash normalised; other x.ai subdomains allowed. + assert_eq!( + validate_xai_base_url("https://staging.x.ai/v1/").unwrap(), + "https://staging.x.ai/v1" + ); + // Plaintext, non-xAI hosts, lookalike suffixes, and garbage all fail. + assert!(validate_xai_base_url("http://api.x.ai/v1").is_err()); + assert!(validate_xai_base_url("https://api.evil.example/v1").is_err()); + assert!(validate_xai_base_url("https://notx.ai/v1").is_err()); + assert!(validate_xai_base_url("https://apix.ai/v1").is_err()); + assert!(validate_xai_base_url("not a url").is_err()); + } + + #[test] + fn xai_request_body_carries_max_tokens_and_tools() { + // Review round-3 F3: the documented OPENAB_AGENT_MAX_TOKENS contract + // must reach the wire. + let tools = vec![ToolDef { + name: "bash".to_string(), + description: "run".to_string(), + input_schema: json!({"type": "object"}), + }]; + let body = xai_request_body("grok-4.5", 4096, "sys", &[], &tools); + assert_eq!(body["model"], "grok-4.5"); + assert_eq!(body["max_tokens"], 4096); + assert_eq!(body["stream"], false); + assert_eq!(body["tools"][0]["function"]["name"], "bash"); + assert_eq!(body["tool_choice"], "auto"); + // No tools → the tool fields are absent entirely. + let body = xai_request_body("grok-4.5", 4096, "sys", &[], &[]); + assert!(body.get("tools").is_none()); + assert!(body.get("tool_choice").is_none()); + } + // ── XaiProvider 401 reactive-refresh loop (review F3) ───────────────── // Deterministic coverage via canned local HTTP servers: no live xAI, no // real credentials. HOME is redirected to a tempdir so auth.json reads and @@ -1731,6 +1817,7 @@ mod tests { let provider = XaiProvider { base_url: chat_url.clone(), model: "grok-4.5".to_string(), + max_tokens: 8192, client: reqwest::Client::new(), }; let events = tokio::runtime::Runtime::new() @@ -1768,6 +1855,7 @@ mod tests { let provider = XaiProvider { base_url: chat_url.clone(), model: "grok-4.5".to_string(), + max_tokens: 8192, client: reqwest::Client::new(), }; let err = tokio::runtime::Runtime::new() From f0d096c113ea265766370e48f3eb7c1e2e6e97ea Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sat, 18 Jul 2026 17:17:33 -0400 Subject: [PATCH 5/9] =?UTF-8?q?fix(openab-agent):=20address=20review=20rou?= =?UTF-8?q?nd-4=20F2=20=E2=80=94=20refresh=20gets=20its=20own=20retry=20al?= =?UTF-8?q?lowance?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rate-limit retries (429/529, capped at 3 with exponential backoff) and the one-time 401 refresh now have independent budgets: a 401 arriving after the rate-limit budget is exhausted still gets its post-refresh request instead of rotating the credential and then failing with a generic 'max retries exceeded'. Terminal errors now always report the actual upstream status. Regression: 429×3 → 401 → refreshed request succeeds with the rotated bearer (canned local servers). --- openab-agent/src/llm.rs | 61 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/openab-agent/src/llm.rs b/openab-agent/src/llm.rs index 8467c43a2..36abacc30 100644 --- a/openab-agent/src/llm.rs +++ b/openab-agent/src/llm.rs @@ -1069,9 +1069,15 @@ impl LlmProvider for XaiProvider { Box::pin(async move { let body = xai_request_body(&self.model, self.max_tokens, system, messages, tools); - let max_retries = 3u32; + // Retry budgets are independent (round-4 F2): rate-limit retries + // are capped, while the one-time 401 refresh always gets its own + // follow-up request — a successful refresh must never be consumed + // by an exhausted budget. Every other outcome returns, so the + // loop is bounded at (rate-limit cap + refresh + terminal). + const MAX_RATE_LIMIT_RETRIES: u32 = 3; + let mut rate_limit_retries = 0u32; let mut refreshed_after_401 = false; - for attempt in 0..=max_retries { + loop { let token = crate::auth::get_valid_token_for(crate::auth::XAI_NAMESPACE).await?; let resp = self .client @@ -1084,8 +1090,12 @@ impl LlmProvider for XaiProvider { .map_err(|e| anyhow!("HTTP request failed: {e}"))?; let status = resp.status(); - if (status.as_u16() == 429 || status.as_u16() == 529) && attempt < max_retries { - let delay = std::time::Duration::from_millis(1000 * 2u64.pow(attempt)); + if (status.as_u16() == 429 || status.as_u16() == 529) + && rate_limit_retries < MAX_RATE_LIMIT_RETRIES + { + let delay = + std::time::Duration::from_millis(1000 * 2u64.pow(rate_limit_retries)); + rate_limit_retries += 1; tokio::time::sleep(delay).await; continue; } @@ -1115,7 +1125,6 @@ impl LlmProvider for XaiProvider { // Chat Completions shape → parse_openai_response's fallback path. return parse_openai_response(&payload); } - Err(anyhow!("xAI API: max retries exceeded")) }) } } @@ -1836,6 +1845,48 @@ mod tests { assert!(refresh_reqs[0].contains("rt1")); } + #[test] + fn xai_chat_rate_limits_then_401_still_gets_refreshed_request() { + // Review round-4 F2: three 429s exhaust the rate-limit budget, then a + // 401 triggers the one-time refresh — the refreshed token must still + // get its follow-up request instead of dying on "max retries exceeded". + let home = tempfile::tempdir().unwrap(); + let (chat_url, chat_handle) = spawn_canned_http(vec![ + http_resp("429 Too Many Requests", r#"{"error":"rate"}"#), + http_resp("429 Too Many Requests", r#"{"error":"rate"}"#), + http_resp("429 Too Many Requests", r#"{"error":"rate"}"#), + http_resp("401 Unauthorized", r#"{"error":"unauthorized"}"#), + http_resp( + "200 OK", + r#"{"choices":[{"message":{"content":"ok"},"finish_reason":"stop"}]}"#, + ), + ]); + let (refresh_url, _refresh_handle) = spawn_canned_http(vec![http_resp( + "200 OK", + r#"{"access_token":"fresh-token","refresh_token":"rt2","expires_in":3600}"#, + )]); + write_xai_auth(home.path(), &refresh_url); + + temp_env::with_var("HOME", Some(home.path().to_str().unwrap()), || { + let provider = XaiProvider { + base_url: chat_url.clone(), + model: "grok-4.5".to_string(), + max_tokens: 8192, + client: reqwest::Client::new(), + }; + let events = tokio::runtime::Runtime::new() + .unwrap() + .block_on(provider.chat("sys", &[], &[])) + .unwrap(); + assert!(matches!(&events[0], LlmEvent::Text(t) if t == "ok")); + }); + + let chat_reqs = chat_handle.join().unwrap(); + assert_eq!(chat_reqs.len(), 5); + // The post-refresh request carries the rotated token. + assert!(chat_reqs[4].contains("Bearer fresh-token")); + } + #[test] fn xai_chat_401_refresh_failure_propagates_relogin_error() { let home = tempfile::tempdir().unwrap(); From 6092c45fdd7f3fa2f721c94916205cff00f4fdf7 Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sun, 19 Jul 2026 14:03:29 +0000 Subject: [PATCH 6/9] docs: document xAI config and provider selection --- docs/native-agent.md | 89 ++++++++++++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 24 deletions(-) diff --git a/docs/native-agent.md b/docs/native-agent.md index 562e49042..c1b3ec824 100644 --- a/docs/native-agent.md +++ b/docs/native-agent.md @@ -29,29 +29,47 @@ env = { OPENAB_AGENT_OPENAI_MODEL = "gpt-5.4-mini" } ### Configuration file (config.json) -A small JSON file next to `auth.json` (default `/config.json`, -overridable with `OPENAB_CONFIG_PATH`) declares the default model and params, so -a deployment can set them in a file instead of only via env vars. **Secrets never -go here** — credentials stay in the locked `auth.json` store. +A small **valid JSON** file next to `auth.json` declares the default model and +parameters, so a deployment can set them in a file instead of only via +environment variables. The default path is `$HOME/.openab/agent/config.json` +(`/home/agent/.openab/agent/config.json` for the `agent` user). Set +`OPENAB_CONFIG_PATH` to override the whole path. **Secrets never go here** — +credentials stay in the locked `auth.json` store. -```jsonc +For example, an xAI deployment can use: + +```json { - "model": "anthropic/claude-sonnet-4-6", // single provider/model string - "max_tokens": 8192 // optional + "model": "xai/grok-4.5", + "max_tokens": 8192 } ``` -Resolution is **env-over-config**: `OPENAB_AGENT_MODEL` / `OPENAB_AGENT_MAX_TOKENS` -override the file, so a pod's injected env stays authoritative over a baked -config. A missing file is fine (empty config); a malformed file is logged and -ignored (the agent then falls back to env / built-in defaults). Unknown keys are -tolerated for forward-compatibility. +The supported fields are `model` (a `provider/model` string) and `max_tokens`. +A missing file is fine (empty config); malformed JSON is logged and ignored, so +the agent falls back to environment variables and built-in defaults. Unknown +keys are tolerated for forward compatibility. + +Provider selection and value precedence are separate but related: + +1. `OPENAB_AGENT_PROVIDER` explicitly selects `anthropic`, `openai`, `codex`, + `xai`, or `grok`. +2. Otherwise, a provider prefix in `OPENAB_AGENT_MODEL` wins (for example, + `xai/grok-4.5`). +3. Otherwise, a provider prefix in `config.json`'s `model` is used. +4. With no explicit provider, auto-detection remains Anthropic → Codex; + xAI is **not** selected merely because an `xai-oauth` token exists. + +`OPENAB_AGENT_MODEL` and `OPENAB_AGENT_MAX_TOKENS` override their config-file +values, so environment variables injected into a pod remain authoritative. +`OPENAB_AGENT_XAI_MODEL` controls the xAI model after xAI has been selected; it +does not by itself enable xAI auto-detection. ## Environment Variables | Variable | Default | Description | |----------|---------|-------------| -| `OPENAB_AGENT_MODEL` | — (required for Anthropic) | Anthropic model id, optionally `provider/`-qualified (e.g. `claude-opus-4-8`, `anthropic/claude-opus-4-8`). No hardcoded default — dateless 4.6+ IDs are fixed canonical IDs that retire each generation, so the agent fails loud if unset rather than pin a model that will eventually 404. Overrides `model` in [config.json](#configuration-file-configjson). | +| `OPENAB_AGENT_MODEL` | — (required for Anthropic) | Model to use, optionally `provider/`-qualified (for example, `anthropic/claude-opus-4-8` or `xai/grok-4.5`). Anthropic has no hardcoded default and fails loud if unset; xAI falls back to `grok-4.5`. Overrides `model` in [config.json](#configuration-file-configjson). | | `OPENAB_AGENT_OPENAI_MODEL` | `gpt-5.4-mini` | Model to use (must be supported by your ChatGPT plan — see [Supported Models](#supported-models-chatgpt-subscription)) | | `OPENAB_AGENT_OPENAI_BASE_URL` | `https://chatgpt.com/backend-api` | API base URL | | `OPENAB_AGENT_XAI_MODEL` | `grok-4.5` | xAI model to use (see [xAI credentials](#xai-credentials-supergrok--x-premium)) | @@ -128,10 +146,33 @@ openab-agent auth xai-device ``` Prints a user code and an `https://auth.x.ai/...` verification link (prefilled -when the server provides one). Tokens are stored under the `xai-oauth` tenant in -`auth.json` and refreshed automatically. Select the provider with -`OPENAB_AGENT_PROVIDER=xai` or a `xai/`-prefixed model (e.g. -`OPENAB_AGENT_MODEL=xai/grok-4.5`); xAI is not part of auto-detection. +when the server provides one). Tokens are stored under the `xai-oauth` namespace +in `auth.json` and refreshed automatically. The default auth file is +`$HOME/.openab/agent/auth.json`; run `openab-agent auth status` to list each +stored provider, a masked token, and its expiry without printing secrets. + +Select xAI explicitly with `OPENAB_AGENT_PROVIDER=xai` (or `grok`) or a +`xai/`- or `grok/`-prefixed model, for example: + +```bash +OPENAB_AGENT_PROVIDER=xai openab-agent +# or use model/config selection: +OPENAB_AGENT_MODEL=xai/grok-4.5 openab-agent +``` + +A config file can make that selection persistent: + +```json +{ + "model": "xai/grok-4.5", + "max_tokens": 8192 +} +``` + +xAI is not part of auto-detection: an `xai-oauth` entry by itself does not make +the agent choose xAI. `OPENAB_AGENT_XAI_MODEL` controls the model after xAI has +been selected; it does not enable xAI selection on its own. The OAuth token and +refresh token stay in `auth.json`, never in `config.json`. ### Adding an OAuth vendor @@ -147,13 +188,13 @@ Place an `AGENTS.md` file in the working directory (`cwd`). It will be prepended ``` /home/agent/ ├── AGENTS.md ← read at session start -├── .openab/ -│ └── agent/ -│ └── auth.json -│ └── skills/ ← skill directories -│ └── my-skill/ -│ └── SKILL.md -└── (your project files) +└── .openab/ + └── agent/ + ├── config.json ← optional model/provider defaults + ├── auth.json ← OAuth credentials; permissions should remain private + └── skills/ ← skill directories + └── my-skill/ + └── SKILL.md ``` ## Skills From 6339f67348c90ec42c6182d3e37266411a2a5571 Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sun, 19 Jul 2026 14:07:46 +0000 Subject: [PATCH 7/9] feat: use xai as canonical auth command --- docs/native-agent.md | 4 +++- openab-agent/src/auth.rs | 6 +++--- openab-agent/src/llm.rs | 6 +++--- openab-agent/src/main.rs | 23 +++++++++++++++++++++-- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/docs/native-agent.md b/docs/native-agent.md index c1b3ec824..8c4705ab8 100644 --- a/docs/native-agent.md +++ b/docs/native-agent.md @@ -142,9 +142,11 @@ flow — no `XAI_API_KEY` to provision, and headless-friendly (approve on any device; ideal for pods via `kubectl exec`): ```bash -openab-agent auth xai-device +openab-agent auth xai ``` +The legacy `auth xai-device` spelling remains accepted as a compatibility alias. + Prints a user code and an `https://auth.x.ai/...` verification link (prefilled when the server provides one). Tokens are stored under the `xai-oauth` namespace in `auth.json` and refreshed automatically. The default auth file is diff --git a/openab-agent/src/auth.rs b/openab-agent/src/auth.rs index 06c7cd724..afbd611df 100644 --- a/openab-agent/src/auth.rs +++ b/openab-agent/src/auth.rs @@ -536,7 +536,7 @@ fn auth_subcommand(namespace: &str) -> &'static str { if namespace == ANTHROPIC_NAMESPACE { "openab-agent auth anthropic-oauth" } else if namespace == XAI_NAMESPACE { - "openab-agent auth xai-device" + "openab-agent auth xai" } else { "openab-agent auth codex-oauth" } @@ -1546,7 +1546,7 @@ pub fn show_status() { if tokens.is_empty() { println!( - "Not authenticated.\nRun: openab-agent auth codex-oauth | openab-agent auth anthropic-oauth | openab-agent auth xai-device" + "Not authenticated.\nRun: openab-agent auth codex-oauth | openab-agent auth anthropic-oauth | openab-agent auth xai" ); return; } @@ -1718,7 +1718,7 @@ mod tests { fn auth_subcommand_per_namespace() { assert_eq!( auth_subcommand(XAI_NAMESPACE), - "openab-agent auth xai-device" + "openab-agent auth xai" ); assert_eq!( auth_subcommand(ANTHROPIC_NAMESPACE), diff --git a/openab-agent/src/llm.rs b/openab-agent/src/llm.rs index 36abacc30..38e0cee24 100644 --- a/openab-agent/src/llm.rs +++ b/openab-agent/src/llm.rs @@ -196,7 +196,7 @@ pub fn select_provider(choice: &str) -> Result, String> { OpenAiProvider::from_auth_store() .map(|p| Box::new(p) as Box) .map_err(|codex_err| format!( - "No credentials: set ANTHROPIC_API_KEY / CLAUDE_CODE_OAUTH_TOKEN, or run `openab-agent auth anthropic-oauth` / `openab-agent auth codex-oauth` / `openab-agent auth xai-device`. ({codex_err})" + "No credentials: set ANTHROPIC_API_KEY / CLAUDE_CODE_OAUTH_TOKEN, or run `openab-agent auth anthropic-oauth` / `openab-agent auth codex-oauth` / `openab-agent auth xai`. ({codex_err})" )) } } @@ -916,7 +916,7 @@ fn validate_xai_base_url(raw: &str) -> Result { impl XaiProvider { /// Create provider using the stored xAI OAuth token from - /// `~/.openab/agent/auth.json` (run `openab-agent auth xai-device` first). + /// `~/.openab/agent/auth.json` (run `openab-agent auth xai` first). pub fn from_auth_store() -> Result { // Just verify tokens exist; the live token is fetched (and refreshed) // per call, mirroring `OpenAiProvider`. @@ -1920,7 +1920,7 @@ mod tests { err.contains("xAI token refresh after HTTP 401 failed"), "got: {err}" ); - assert!(err.contains("openab-agent auth xai-device"), "got: {err}"); + assert!(err.contains("openab-agent auth xai"), "got: {err}"); }); } } diff --git a/openab-agent/src/main.rs b/openab-agent/src/main.rs index 091ced816..9605d5ebd 100644 --- a/openab-agent/src/main.rs +++ b/openab-agent/src/main.rs @@ -94,7 +94,8 @@ enum AuthProvider { no_browser: bool, }, /// xAI SuperGrok / X Premium via device code (RFC 8628, headless-friendly) - XaiDevice, + #[command(visible_alias = "xai-device")] + Xai, /// Show stored credentials Status, } @@ -133,7 +134,7 @@ async fn main() { std::process::exit(1); } } - AuthProvider::XaiDevice => { + AuthProvider::Xai => { if let Err(e) = auth::login_xai_device_flow().await { eprintln!("❌ Authentication failed: {e}"); std::process::exit(1); @@ -162,3 +163,21 @@ async fn main() { }, } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn xai_auth_accepts_canonical_and_legacy_alias() { + for name in ["xai", "xai-device"] { + let cli = Cli::try_parse_from(["openab-agent", "auth", name]).unwrap(); + assert!(matches!( + cli.command, + Some(Commands::Auth { + provider: AuthProvider::Xai + }) + )); + } + } +} From 8fefcb018827e8a4ffda17fdd355200bd377b4b4 Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sun, 19 Jul 2026 14:09:12 +0000 Subject: [PATCH 8/9] refactor: remove xai-device auth alias --- docs/native-agent.md | 2 -- openab-agent/src/main.rs | 19 ++++++++----------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/docs/native-agent.md b/docs/native-agent.md index 8c4705ab8..4be2efdc3 100644 --- a/docs/native-agent.md +++ b/docs/native-agent.md @@ -145,8 +145,6 @@ device; ideal for pods via `kubectl exec`): openab-agent auth xai ``` -The legacy `auth xai-device` spelling remains accepted as a compatibility alias. - Prints a user code and an `https://auth.x.ai/...` verification link (prefilled when the server provides one). Tokens are stored under the `xai-oauth` namespace in `auth.json` and refreshed automatically. The default auth file is diff --git a/openab-agent/src/main.rs b/openab-agent/src/main.rs index 9605d5ebd..555e535fd 100644 --- a/openab-agent/src/main.rs +++ b/openab-agent/src/main.rs @@ -94,7 +94,6 @@ enum AuthProvider { no_browser: bool, }, /// xAI SuperGrok / X Premium via device code (RFC 8628, headless-friendly) - #[command(visible_alias = "xai-device")] Xai, /// Show stored credentials Status, @@ -169,15 +168,13 @@ mod tests { use super::*; #[test] - fn xai_auth_accepts_canonical_and_legacy_alias() { - for name in ["xai", "xai-device"] { - let cli = Cli::try_parse_from(["openab-agent", "auth", name]).unwrap(); - assert!(matches!( - cli.command, - Some(Commands::Auth { - provider: AuthProvider::Xai - }) - )); - } + fn xai_auth_accepts_canonical_command() { + let cli = Cli::try_parse_from(["openab-agent", "auth", "xai"]).unwrap(); + assert!(matches!( + cli.command, + Some(Commands::Auth { + provider: AuthProvider::Xai + }) + )); } } From 6a4566063e5825c1a8968298ce566a62e07229c9 Mon Sep 17 00:00:00 2001 From: chaodu-agent <274062505+chaodu-agent@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:29:39 +0000 Subject: [PATCH 9/9] fix(openab-agent): apply rustfmt to xAI auth test --- openab-agent/src/auth.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/openab-agent/src/auth.rs b/openab-agent/src/auth.rs index afbd611df..7a8f2fd38 100644 --- a/openab-agent/src/auth.rs +++ b/openab-agent/src/auth.rs @@ -1716,10 +1716,7 @@ mod tests { #[test] fn auth_subcommand_per_namespace() { - assert_eq!( - auth_subcommand(XAI_NAMESPACE), - "openab-agent auth xai" - ); + assert_eq!(auth_subcommand(XAI_NAMESPACE), "openab-agent auth xai"); assert_eq!( auth_subcommand(ANTHROPIC_NAMESPACE), "openab-agent auth anthropic-oauth"