inference: oauth (chatgpt-style) for openai llm provider (#1953)#2265
Conversation
…i#1953) Add Codex-style PKCE OAuth for the openai provider slug, wire bearer lookup and JSON-RPC controllers, and surface sign-in during onboarding API keys step. Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughAdds OpenAI (ChatGPT/Codex) OAuth: frontend onboarding sign-in UI, backend PKCE/state OAuth flow, token persistence and refresh, RPC endpoints and schemas, provider lookup integration, and tests. ChangesOpenAI OAuth Authentication
Sequence DiagramsequenceDiagram
participant UI as Onboarding UI
participant Core as Core RPC (callCoreRpc)
participant Ops as inference ops
participant Flow as openai_oauth::flow
participant TokenEndpoint as OpenAI token endpoint
participant Store as AuthProfilesStore
UI->>Core: inference_openai_oauth_start()
Core->>Ops: inference_openai_oauth_start
Ops->>Flow: start_openai_oauth()
Flow->>UI: {auth_url, state, redirect_uri}
UI->>Core: inference_openai_oauth_complete(callback_url)
Core->>Ops: inference_openai_oauth_complete(callback_url)
Ops->>Flow: complete_openai_oauth(callback_url)
Flow->>TokenEndpoint: POST token exchange (code, PKCE, client)
TokenEndpoint-->>Flow: token JSON
Flow->>Store: persist_openai_oauth_token(token)
Flow->>Ops: return connected payload
Ops->>Core: RpcOutcome
Core->>UI: connected status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/pages/onboarding/steps/ApiKeysStep.tsx (1)
253-259:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate Tailwind class removes dark mode hover effect.
The className has both
dark:text-neutral-400anddark:text-neutral-200, where the latter overrides the former. This makes the skip button's dark mode base color the same as the hover color, eliminating the hover feedback. Remove the trailing duplicate.🧹 Suggested fix
<button type="button" onClick={onSkip} disabled={saving} - className="text-xs text-stone-500 dark:text-neutral-400 hover:text-stone-700 dark:hover:text-neutral-200 dark:text-neutral-200 underline disabled:opacity-50"> + className="text-xs text-stone-500 dark:text-neutral-400 hover:text-stone-700 dark:hover:text-neutral-200 underline disabled:opacity-50"> {t('onboarding.apiKeys.skipForNow')} </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/onboarding/steps/ApiKeysStep.tsx` around lines 253 - 259, The skip button in ApiKeysStep has duplicate dark mode classes (dark:text-neutral-400 and dark:text-neutral-200) causing the hover to be no-op; edit the button's className in ApiKeysStep (the <button> with onClick={onSkip}) to remove the trailing duplicate dark:text-neutral-200 so the base dark color stays neutral-400 and the hover dark:hover:text-neutral-200 remains effective.
🧹 Nitpick comments (3)
app/src/pages/onboarding/steps/ApiKeysStep.tsx (1)
172-184: 💤 Low valueAdd
aria-labelto the callback URL input for accessibility.The callback URL input lacks a proper label association. While there's hint text above it, screen readers may not announce the input's purpose. Consider adding an
aria-labelattribute.♿ Suggested fix
<input data-testid="onboarding-openai-oauth-callback-input" type="text" + aria-label="Redirect URL from ChatGPT sign-in" autoComplete="off" spellCheck={false}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/onboarding/steps/ApiKeysStep.tsx` around lines 172 - 184, The input for the OAuth callback URL (the element bound to value={oauthCallbackUrl} and onChange calling setOauthCallbackUrl) is missing an accessible name; add an aria-label attribute (for example aria-label="OpenAI OAuth callback URL") to that input (data-testid="onboarding-openai-oauth-callback-input") so screen readers can announce its purpose while keeping existing placeholder and behavior unchanged.src/openhuman/inference/schemas_tests.rs (1)
8-8: ⚡ Quick winHarden stability coverage for the newly added OAuth RPC names.
Line 8 updates only a minimum count, which won’t catch accidental rename/removal of a specific OAuth endpoint. Add explicit assertions for the four new
openai_oauth_*function names ininference_schema_function_names_are_stable.✅ Suggested test additions
fn inference_schema_function_names_are_stable() { let functions: Vec<&str> = all_controller_schemas() .into_iter() .map(|schema| schema.function) .collect(); @@ assert!(functions.contains(&"diagnostics")); + assert!(functions.contains(&"openai_oauth_start")); + assert!(functions.contains(&"openai_oauth_complete")); + assert!(functions.contains(&"openai_oauth_status")); + assert!(functions.contains(&"openai_oauth_disconnect")); assert!(functions.contains(&"prompt"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/inference/schemas_tests.rs` at line 8, The test inference_schema_function_names_are_stable currently only checks declared.len() >= 20 which won't catch renames/removals; modify that test to also assert that the declared collection contains each of the four OAuth RPC names (e.g., "openai_oauth_connect", "openai_oauth_refresh", "openai_oauth_revoke", "openai_oauth_callback") by adding explicit assertions using declared.contains(<name>) or equivalent checks so the test fails if any specific OAuth endpoint name is changed or removed.src/openhuman/inference/openai_oauth/flow.rs (1)
155-166: ⚡ Quick winAdd debug/trace logs on error paths and state transitions.
State mismatch, token-exchange HTTP failures, and status/disconnect branches currently return without structured debug/trace logging context.
Suggested logging additions
pub async fn complete_openai_oauth( config: &Config, callback_input: &str, ) -> Result<serde_json::Value, String> { + log::debug!("{LOG_PREFIX} complete enter"); let pending = read_pending(config)? .ok_or_else(|| "no pending OAuth session; call openai_oauth_start first".to_string())?; @@ let (code, returned_state) = parse_callback_input(callback_input)?; if returned_state != pending.state { + log::debug!("{LOG_PREFIX} complete state_mismatch; clearing pending"); clear_pending(config); return Err("OAuth state mismatch — try connecting again".to_string()); } @@ pub fn openai_oauth_status(config: &Config) -> Result<OpenAiOAuthStatusResult, String> { + log::debug!("{LOG_PREFIX} status enter"); @@ pub fn disconnect_openai_oauth(config: &Config) -> Result<serde_json::Value, String> { + log::debug!("{LOG_PREFIX} disconnect enter"); @@ if !resp.status().is_success() { let status = resp.status(); let body = resp.text().await.unwrap_or_default(); + log::debug!("{LOG_PREFIX} token_exchange http_error status={status}"); return Err(format!("HTTP {status}: {body}")); }As per coding guidelines: "Use
log/tracingatdebugortracelevel on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone."Also applies to: 190-234, 286-290
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/inference/openai_oauth/flow.rs` around lines 155 - 166, In complete_openai_oauth, add structured debug/trace logs for RPC entry/exit and key state transitions and error branches: log when reading pending via read_pending (including absent pending), after parse_callback_input with code and returned_state, on state mismatch before/after calling clear_pending, and around the token-exchange HTTP call (log request/response status and errors) and any status/disconnect branches that currently return; use tracing::debug!/trace! (or log::debug!) and include contextual fields (e.g., pending.state, returned_state, error) to make failures and flow visible in the functions referenced (complete_openai_oauth, read_pending, parse_callback_input, clear_pending and the token exchange handlers).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/inference/openai_oauth/flow.rs`:
- Around line 278-284: The token exchange uses reqwest::Client::new() without a
timeout which can hang; change to build a Client with a timeout (e.g.,
reqwest::Client::builder().timeout(Duration::from_secs(X)).build() ) and use
that client for the POST that assigns resp; ensure you add the
std::time::Duration import (or use a configurable timeout from config) and
handle the builder's Result, mapping errors the same way as before so the token
exchange path respects the timeout.
In `@src/openhuman/inference/openai_oauth/store.rs`:
- Line 116: The upsert of the refreshed profile is currently ignored; change the
call to auth_profiles_store(config).upsert_profile(profile, true) to check its
Result and log any Err at warn level so refresh persistence failures are
visible; include contextual info (e.g. profile id or subject and the error) in
the warn message and retain successful path behavior. Use the existing logging
facility (e.g. warn!/tracing or log::warn) and avoid panicking—just surface the
error so it can be investigated.
- Around line 55-62: The sync helper try_refresh_oauth_token currently calls
handle.block_on(fut) directly which can deadlock when invoked from an async
Tokio context; instead wrap the blocking call inside tokio::task::block_in_place
so the runtime can move the task to a blocking thread. Update
try_refresh_oauth_token to, when a Handle is available (the existing
tokio::runtime::Handle::try_current() branch), execute the blocking runtime call
inside tokio::task::block_in_place(|| { handle.block_on(fut) }) and then map the
result to a string error as before; keep the existing fallback Err(...) when no
runtime handle is found. Reference: try_refresh_oauth_token,
motosan_ai_oauth::refresh, tokio::runtime::Handle and
tokio::task::block_in_place.
In `@src/openhuman/inference/ops.rs`:
- Around line 313-353: The OpenAI OAuth RPC handlers
(inference_openai_oauth_start, inference_openai_oauth_complete,
inference_openai_oauth_status, inference_openai_oauth_disconnect) lack the
standard RPC lifecycle tracing (start/ok/error) used elsewhere; add trace/debug
logs at RPC entry, on success, and on error following the same start/ok/error
pattern used by other ops so failures are observable in production.
Specifically, at the top of each function emit a "start" trace with relevant
input (e.g., callback_url for complete), then on success emit the existing
RpcOutcome::single_log message as the "ok" trace, and on any error path log the
error with an "error" trace including the error string and any useful state
(e.g., start.state/auth_url or status.connected/profile_id). Use the same
logging level and message format as other RPCs to remain consistent.
---
Outside diff comments:
In `@app/src/pages/onboarding/steps/ApiKeysStep.tsx`:
- Around line 253-259: The skip button in ApiKeysStep has duplicate dark mode
classes (dark:text-neutral-400 and dark:text-neutral-200) causing the hover to
be no-op; edit the button's className in ApiKeysStep (the <button> with
onClick={onSkip}) to remove the trailing duplicate dark:text-neutral-200 so the
base dark color stays neutral-400 and the hover dark:hover:text-neutral-200
remains effective.
---
Nitpick comments:
In `@app/src/pages/onboarding/steps/ApiKeysStep.tsx`:
- Around line 172-184: The input for the OAuth callback URL (the element bound
to value={oauthCallbackUrl} and onChange calling setOauthCallbackUrl) is missing
an accessible name; add an aria-label attribute (for example aria-label="OpenAI
OAuth callback URL") to that input
(data-testid="onboarding-openai-oauth-callback-input") so screen readers can
announce its purpose while keeping existing placeholder and behavior unchanged.
In `@src/openhuman/inference/openai_oauth/flow.rs`:
- Around line 155-166: In complete_openai_oauth, add structured debug/trace logs
for RPC entry/exit and key state transitions and error branches: log when
reading pending via read_pending (including absent pending), after
parse_callback_input with code and returned_state, on state mismatch
before/after calling clear_pending, and around the token-exchange HTTP call (log
request/response status and errors) and any status/disconnect branches that
currently return; use tracing::debug!/trace! (or log::debug!) and include
contextual fields (e.g., pending.state, returned_state, error) to make failures
and flow visible in the functions referenced (complete_openai_oauth,
read_pending, parse_callback_input, clear_pending and the token exchange
handlers).
In `@src/openhuman/inference/schemas_tests.rs`:
- Line 8: The test inference_schema_function_names_are_stable currently only
checks declared.len() >= 20 which won't catch renames/removals; modify that test
to also assert that the declared collection contains each of the four OAuth RPC
names (e.g., "openai_oauth_connect", "openai_oauth_refresh",
"openai_oauth_revoke", "openai_oauth_callback") by adding explicit assertions
using declared.contains(<name>) or equivalent checks so the test fails if any
specific OAuth endpoint name is changed or removed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 884a5997-3c8d-49fd-972a-93c6d0bb68e8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlapp/src/pages/onboarding/steps/ApiKeysStep.tsxapp/src/pages/onboarding/steps/__tests__/ApiKeysStep.test.tsxsrc/openhuman/inference/mod.rssrc/openhuman/inference/openai_oauth/config.rssrc/openhuman/inference/openai_oauth/flow.rssrc/openhuman/inference/openai_oauth/flow_tests.rssrc/openhuman/inference/openai_oauth/mod.rssrc/openhuman/inference/openai_oauth/store.rssrc/openhuman/inference/ops.rssrc/openhuman/inference/provider/factory.rssrc/openhuman/inference/schemas.rssrc/openhuman/inference/schemas_tests.rs
| let resp = reqwest::Client::new() | ||
| .post(config.token_url) | ||
| .header("Accept", "application/json") | ||
| .form(¶ms) | ||
| .send() | ||
| .await | ||
| .map_err(|e| e.to_string())?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether exchange_authorization_code sets an explicit timeout.
rg -n -C3 'async fn exchange_authorization_code|Client::new\(|Client::builder|timeout\(' src/openhuman/inference/openai_oauth/flow.rsRepository: tinyhumansai/openhuman
Length of output: 457
Add an explicit timeout to the token exchange HTTP call.
Line 278 creates a reqwest client without a timeout, allowing this path to block indefinitely on network stalls and tie up the OAuth completion flow.
Suggested fix
+use std::time::Duration;
+
+const OAUTH_HTTP_TIMEOUT_SECS: u64 = 20;
+
async fn exchange_authorization_code(
config: &motosan_ai_oauth::OAuthConfig,
code: &str,
@@
- let resp = reqwest::Client::new()
- .post(config.token_url)
+ let client = reqwest::Client::builder()
+ .timeout(Duration::from_secs(OAUTH_HTTP_TIMEOUT_SECS))
+ .build()
+ .map_err(|e| e.to_string())?;
+
+ let resp = client
+ .post(config.token_url)
.header("Accept", "application/json")
.form(¶ms)
.send()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let resp = reqwest::Client::new() | |
| .post(config.token_url) | |
| .header("Accept", "application/json") | |
| .form(¶ms) | |
| .send() | |
| .await | |
| .map_err(|e| e.to_string())?; | |
| use std::time::Duration; | |
| const OAUTH_HTTP_TIMEOUT_SECS: u64 = 20; |
| let resp = reqwest::Client::new() | |
| .post(config.token_url) | |
| .header("Accept", "application/json") | |
| .form(¶ms) | |
| .send() | |
| .await | |
| .map_err(|e| e.to_string())?; | |
| let client = reqwest::Client::builder() | |
| .timeout(Duration::from_secs(OAUTH_HTTP_TIMEOUT_SECS)) | |
| .build() | |
| .map_err(|e| e.to_string())?; | |
| let resp = client | |
| .post(config.token_url) | |
| .header("Accept", "application/json") | |
| .form(¶ms) | |
| .send() | |
| .await | |
| .map_err(|e| e.to_string())?; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/inference/openai_oauth/flow.rs` around lines 278 - 284, The
token exchange uses reqwest::Client::new() without a timeout which can hang;
change to build a Client with a timeout (e.g.,
reqwest::Client::builder().timeout(Duration::from_secs(X)).build() ) and use
that client for the POST that assigns resp; ensure you add the
std::time::Duration import (or use a configurable timeout from config) and
handle the builder's Result, mapping errors the same way as before so the token
exchange path respects the timeout.
| fn try_refresh_oauth_token(refresh: &str) -> Result<Token, String> { | ||
| let cfg = codex_oauth_config(); | ||
| let fut = motosan_ai_oauth::refresh(&cfg, refresh); | ||
| if let Ok(handle) = tokio::runtime::Handle::try_current() { | ||
| return handle.block_on(fut).map_err(|e| e.to_string()); | ||
| } | ||
| Err("tokio runtime required to refresh openai oauth token".to_string()) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
tokio block_in_place vs block_on deadlock
💡 Result:
In Tokio, deadlocks involving block_in_place vs block_on are usually caused by “reentering async while holding something the async side needs” (e.g., a mutex/permit/waker) or by assuming that work can keep progressing while a particular thread is synchronously blocked. 1) What block_in_place actually guarantees - block_in_place runs a blocking closure “on the current thread without blocking the executor”, and it tells the executor the task is about to block so the executor can hand off other tasks to another worker thread before that happens [1][2]. - However, any other code running concurrently in the same async task is suspended during the call to block_in_place (so your task won’t make progress on other futures while it’s inside the blocking closure) [1][2]. - code inside block_in_place may call block_on to reenter async [1][2]. So block_in_place is not “magically safe”; it changes scheduling on the current worker thread and can still stall progress depending on what the blocked closure is waiting on. 2) Why combining block_in_place + block_on can deadlock A concrete example is documented in Tokio’s issue tracker: - Issue #7892 describes a Future that first observes Pending after polling a mutex-lock future while the mutex is held, and on a later poll uses task::block_in_place + Handle::block_on to await Mutex::lock. - It states this reliably deadlocks the process, and explains that Tokio’s semaphore logic makes the task unable to acquire the mutex in the second poll [3]. Another class of deadlock is “waker/task wake-up ordering” leading to indefinite stalling: - Tokio issue #7877 reports a scenario where deferred wakers are woken only after block_in_place returns, which can lead to indefinite stalling in code that relies on those wakers being triggered before the blocking call finishes [4]. 3) Practical guidance: when block_in_place is not advisable (vs spawn_blocking) - Tokio’s own docs effectively warn you that block_in_place is a footgun if you have other concurrent async work in the same task (because it suspends it for the duration) [1][2]. - In general, for “blocking the thread” work, Tokio recommends using spawn_blocking instead of block_in_place to avoid these single-task suspension issues [1][2]. - block_in_place also cannot be used on a current_thread runtime (it panics) [1][2]. 4) Does “block_in_place vs block_on” itself cause deadlock? - block_on (when called from within async) is fundamentally different: it blocks the thread to drive async to completion. - block_in_place is specifically intended as the escape hatch when you’re inside async and need to run a blocking closure, with the executor able to move other tasks off the thread. But if the closure (or the block_on you run inside it) waits for async work that, in turn, needs progress from the same runtime resources that are currently blocked/stalled (e.g., mutex acquisition controlled by runtime permits/semaphores, or wake-ups deferred until after the blocking call), you can deadlock [3][4]. Bottom line - block_in_place avoids starving other independently spawned tasks by coordinating with the executor [1][2], but it can still deadlock (or stall forever) if the async you reenter with block_on requires progress that can’t happen while you’re in the blocking closure—especially when mutex/permits/waker timing are involved [3][4]. - Prefer spawn_blocking for truly blocking work; use block_in_place only when you understand the interaction between the blocking section and what the awaited async will need to make progress [1][2].
Citations:
- 1: https://docs.rs/tokio/1/tokio/task/fn.block_in_place.html
- 2: https://docs.rs/tokio/latest/tokio/task/fn.block_in_place.html
- 3:
block_in_place+block_on+tokio::sync::Mutexmay lead to a deadlock tokio-rs/tokio#7892 - 4: In
block_in_place, deferred wakers are woken up after the blocking call and not before tokio-rs/tokio#7877
🏁 Script executed:
# Search for calls to try_refresh_oauth_token
rg "try_refresh_oauth_token" --type rust -B 3 -A 3Repository: tinyhumansai/openhuman
Length of output: 1353
🏁 Script executed:
# Also search for all references to the function in store.rs and related files
rg "lookup_openai_bearer_token" --type rust -B 2 -A 2Repository: tinyhumansai/openhuman
Length of output: 2747
🏁 Script executed:
# Check the broader context - find where these functions are called
rg "refresh_oauth_token|lookup_openai_bearer_token" --type rust -lRepository: tinyhumansai/openhuman
Length of output: 251
🏁 Script executed:
# Examine the factory.rs file to see the context of lookup_key_for_slug
cat -n src/openhuman/inference/provider/factory.rs | head -50Repository: tinyhumansai/openhuman
Length of output: 2677
🏁 Script executed:
# Check if there are async functions that call lookup_key_for_slug
rg "lookup_key_for_slug" --type rust -B 5 -A 5Repository: tinyhumansai/openhuman
Length of output: 2824
🏁 Script executed:
# Check the store.rs file around line 116 where try_refresh_oauth_token is called
sed -n '110,125p' src/openhuman/inference/openai_oauth/store.rsRepository: tinyhumansai/openhuman
Length of output: 656
🏁 Script executed:
# Check the context in ops.rs where lookup_key_for_slug is called
rg "lookup_key_for_slug|list_models" src/openhuman/inference/provider/ops.rs -B 10 -A 2 | head -40Repository: tinyhumansai/openhuman
Length of output: 1282
🏁 Script executed:
# Find all async functions in the inference module that might call into this chain
rg "async fn" src/openhuman/inference --type rust -l | head -10Repository: tinyhumansai/openhuman
Length of output: 500
🏁 Script executed:
# Check if there's a Tokio runtime configuration in the project
rg "tokio::runtime|#\[tokio::main\]" --type rustRepository: tinyhumansai/openhuman
Length of output: 5186
Confirmed: deadlock risk from block_on() on current Tokio runtime.
The call chain list_configured_models (async in ops.rs) → lookup_key_for_slug (sync) → lookup_openai_bearer_token (sync) → try_refresh_oauth_token (sync) invokes handle.block_on(fut) on the current runtime. Calling block_on from within an async context on the same Tokio runtime thread will deadlock—the blocking call waits for the future, but that future cannot progress because the thread is blocked.
Use tokio::task::block_in_place to allow the executor to move other tasks off the blocked thread, preventing starvation of other work on the runtime:
🔧 Fix using block_in_place
fn try_refresh_oauth_token(refresh: &str) -> Result<Token, String> {
let cfg = codex_oauth_config();
+ let refresh = refresh.to_string();
- let fut = motosan_ai_oauth::refresh(&cfg, refresh);
- if let Ok(handle) = tokio::runtime::Handle::try_current() {
- return handle.block_on(fut).map_err(|e| e.to_string());
+ if let Ok(handle) = tokio::runtime::Handle::try_current() {
+ return tokio::task::block_in_place(|| {
+ handle.block_on(motosan_ai_oauth::refresh(&cfg, &refresh))
+ })
+ .map_err(|e| e.to_string());
}
Err("tokio runtime required to refresh openai oauth token".to_string())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn try_refresh_oauth_token(refresh: &str) -> Result<Token, String> { | |
| let cfg = codex_oauth_config(); | |
| let fut = motosan_ai_oauth::refresh(&cfg, refresh); | |
| if let Ok(handle) = tokio::runtime::Handle::try_current() { | |
| return handle.block_on(fut).map_err(|e| e.to_string()); | |
| } | |
| Err("tokio runtime required to refresh openai oauth token".to_string()) | |
| } | |
| fn try_refresh_oauth_token(refresh: &str) -> Result<Token, String> { | |
| let cfg = codex_oauth_config(); | |
| let refresh = refresh.to_string(); | |
| if let Ok(handle) = tokio::runtime::Handle::try_current() { | |
| return tokio::task::block_in_place(|| { | |
| handle.block_on(motosan_ai_oauth::refresh(&cfg, &refresh)) | |
| }) | |
| .map_err(|e| e.to_string()); | |
| } | |
| Err("tokio runtime required to refresh openai oauth token".to_string()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/inference/openai_oauth/store.rs` around lines 55 - 62, The sync
helper try_refresh_oauth_token currently calls handle.block_on(fut) directly
which can deadlock when invoked from an async Tokio context; instead wrap the
blocking call inside tokio::task::block_in_place so the runtime can move the
task to a blocking thread. Update try_refresh_oauth_token to, when a Handle is
available (the existing tokio::runtime::Handle::try_current() branch), execute
the blocking runtime call inside tokio::task::block_in_place(|| {
handle.block_on(fut) }) and then map the result to a string error as before;
keep the existing fallback Err(...) when no runtime handle is found. Reference:
try_refresh_oauth_token, motosan_ai_oauth::refresh, tokio::runtime::Handle and
tokio::task::block_in_place.
| Ok(fresh) => { | ||
| token_set = token_set_from_codex(&fresh); | ||
| profile.token_set = Some(token_set.clone()); | ||
| let _ = auth_profiles_store(config).upsert_profile(profile, true); |
There was a problem hiding this comment.
Silent discard of upsert error may hide refresh persistence failures.
If the profile upsert fails after a successful token refresh, the fresh token is lost on restart, forcing another refresh or re-auth. Consider logging this at warn level.
🛡️ Proposed fix to log upsert errors
- let _ = auth_profiles_store(config).upsert_profile(profile, true);
+ if let Err(e) = auth_profiles_store(config).upsert_profile(profile, true) {
+ log::warn!("{LOG_PREFIX} failed to persist refreshed token: {e}");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let _ = auth_profiles_store(config).upsert_profile(profile, true); | |
| if let Err(e) = auth_profiles_store(config).upsert_profile(profile, true) { | |
| log::warn!("{LOG_PREFIX} failed to persist refreshed token: {e}"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/inference/openai_oauth/store.rs` at line 116, The upsert of the
refreshed profile is currently ignored; change the call to
auth_profiles_store(config).upsert_profile(profile, true) to check its Result
and log any Err at warn level so refresh persistence failures are visible;
include contextual info (e.g. profile id or subject and the error) in the warn
message and retain successful path behavior. Use the existing logging facility
(e.g. warn!/tracing or log::warn) and avoid panicking—just surface the error so
it can be investigated.
| pub async fn inference_openai_oauth_start(config: &Config) -> Result<RpcOutcome<Value>, String> { | ||
| let start = crate::openhuman::inference::openai_oauth::start_openai_oauth(config)?; | ||
| Ok(RpcOutcome::single_log( | ||
| json!({ | ||
| "authUrl": start.auth_url, | ||
| "state": start.state, | ||
| "redirectUri": start.redirect_uri, | ||
| }), | ||
| "openai oauth authorize url ready", | ||
| )) | ||
| } | ||
|
|
||
| pub async fn inference_openai_oauth_complete( | ||
| config: &Config, | ||
| callback_url: &str, | ||
| ) -> Result<RpcOutcome<Value>, String> { | ||
| let result = | ||
| crate::openhuman::inference::openai_oauth::complete_openai_oauth(config, callback_url) | ||
| .await?; | ||
| Ok(RpcOutcome::single_log(result, "openai oauth connected")) | ||
| } | ||
|
|
||
| pub async fn inference_openai_oauth_status(config: &Config) -> Result<RpcOutcome<Value>, String> { | ||
| let status = crate::openhuman::inference::openai_oauth::openai_oauth_status(config)?; | ||
| Ok(RpcOutcome::single_log( | ||
| json!({ | ||
| "connected": status.connected, | ||
| "profileId": status.profile_id, | ||
| "expiresAt": status.expires_at, | ||
| "authMethod": status.auth_method, | ||
| }), | ||
| "openai oauth status", | ||
| )) | ||
| } | ||
|
|
||
| pub async fn inference_openai_oauth_disconnect( | ||
| config: &Config, | ||
| ) -> Result<RpcOutcome<Value>, String> { | ||
| let result = crate::openhuman::inference::openai_oauth::disconnect_openai_oauth(config)?; | ||
| Ok(RpcOutcome::single_log(result, "openai oauth disconnected")) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add RPC lifecycle tracing for OpenAI OAuth handlers.
Line 313 through Line 353 skip the start/ok/error tracing pattern used by the rest of this RPC surface, which makes OAuth flow failures much harder to diagnose in production logs.
🔧 Proposed logging-aligned patch
pub async fn inference_openai_oauth_start(config: &Config) -> Result<RpcOutcome<Value>, String> {
- let start = crate::openhuman::inference::openai_oauth::start_openai_oauth(config)?;
- Ok(RpcOutcome::single_log(
+ debug!("{LOG_PREFIX} openai_oauth_start:start");
+ let result = crate::openhuman::inference::openai_oauth::start_openai_oauth(config).map(|start| {
+ RpcOutcome::single_log(
json!({
"authUrl": start.auth_url,
"state": start.state,
"redirectUri": start.redirect_uri,
}),
"openai oauth authorize url ready",
- ))
+ )});
+ match &result {
+ Ok(_) => debug!("{LOG_PREFIX} openai_oauth_start:ok"),
+ Err(err) => error!(error = %err, "{LOG_PREFIX} openai_oauth_start:error"),
+ }
+ result
}
pub async fn inference_openai_oauth_complete(
config: &Config,
callback_url: &str,
) -> Result<RpcOutcome<Value>, String> {
- let result =
- crate::openhuman::inference::openai_oauth::complete_openai_oauth(config, callback_url)
- .await?;
- Ok(RpcOutcome::single_log(result, "openai oauth connected"))
+ debug!(callback_len = callback_url.len(), "{LOG_PREFIX} openai_oauth_complete:start");
+ let result = crate::openhuman::inference::openai_oauth::complete_openai_oauth(config, callback_url)
+ .await
+ .map(|payload| RpcOutcome::single_log(payload, "openai oauth connected"));
+ match &result {
+ Ok(_) => debug!("{LOG_PREFIX} openai_oauth_complete:ok"),
+ Err(err) => error!(error = %err, "{LOG_PREFIX} openai_oauth_complete:error"),
+ }
+ result
}
pub async fn inference_openai_oauth_status(config: &Config) -> Result<RpcOutcome<Value>, String> {
- let status = crate::openhuman::inference::openai_oauth::openai_oauth_status(config)?;
- Ok(RpcOutcome::single_log(
+ debug!("{LOG_PREFIX} openai_oauth_status:start");
+ let result = crate::openhuman::inference::openai_oauth::openai_oauth_status(config).map(|status| {
+ RpcOutcome::single_log(
json!({
"connected": status.connected,
"profileId": status.profile_id,
"expiresAt": status.expires_at,
"authMethod": status.auth_method,
}),
"openai oauth status",
- ))
+ )});
+ match &result {
+ Ok(_) => debug!("{LOG_PREFIX} openai_oauth_status:ok"),
+ Err(err) => error!(error = %err, "{LOG_PREFIX} openai_oauth_status:error"),
+ }
+ result
}
pub async fn inference_openai_oauth_disconnect(
config: &Config,
) -> Result<RpcOutcome<Value>, String> {
- let result = crate::openhuman::inference::openai_oauth::disconnect_openai_oauth(config)?;
- Ok(RpcOutcome::single_log(result, "openai oauth disconnected"))
+ debug!("{LOG_PREFIX} openai_oauth_disconnect:start");
+ let result = crate::openhuman::inference::openai_oauth::disconnect_openai_oauth(config)
+ .map(|payload| RpcOutcome::single_log(payload, "openai oauth disconnected"));
+ match &result {
+ Ok(_) => debug!("{LOG_PREFIX} openai_oauth_disconnect:ok"),
+ Err(err) => error!(error = %err, "{LOG_PREFIX} openai_oauth_disconnect:error"),
+ }
+ result
}As per coding guidelines: “Use log / tracing at debug or trace level on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/openhuman/inference/ops.rs` around lines 313 - 353, The OpenAI OAuth RPC
handlers (inference_openai_oauth_start, inference_openai_oauth_complete,
inference_openai_oauth_status, inference_openai_oauth_disconnect) lack the
standard RPC lifecycle tracing (start/ok/error) used elsewhere; add trace/debug
logs at RPC entry, on success, and on error following the same start/ok/error
pattern used by other ops so failures are observable in production.
Specifically, at the top of each function emit a "start" trace with relevant
input (e.g., callback_url for complete), then on success emit the existing
RpcOutcome::single_log message as the "ok" trace, and on any error path log the
error with an "error" trace including the error string and any useful state
(e.g., start.state/auth_url or status.connected/profile_id). Use the same
logging level and message format as other RPCs to remain consistent.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/inference/openai_oauth/flow.rs (1)
286-290: ⚡ Quick winAdd a debug log on token-exchange failure path.
This branch returns a surfaced error but misses diagnostic context, which makes triage harder.
Suggested patch
if !resp.status().is_success() { let status = resp.status(); let body = resp.text().await.unwrap_or_default(); + log::debug!( + "{LOG_PREFIX} token exchange failed status={} body_len={}", + status, + body.len() + ); return Err(format!("HTTP {status}: {body}")); }As per coding guidelines: Use
log/tracingatdebugortracelevel on RPC entry and exit, error paths, state transitions, and hard-to-infer branches.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/inference/openai_oauth/flow.rs` around lines 286 - 290, The error path in the token-exchange HTTP response handling (the block checking resp.status().is_success()) lacks diagnostic logging; modify this branch in flow.rs (where resp, status and body are computed) to emit a debug-level log/tracing event containing the HTTP status and response body before returning the Err so callers/ops can triage (e.g., call debug!(... or tracing::debug!(... ) with status = %status, body = %body) and keep the existing Err(format!(...)) return unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/inference/openai_oauth/flow.rs`:
- Around line 286-290: The error path in the token-exchange HTTP response
handling (the block checking resp.status().is_success()) lacks diagnostic
logging; modify this branch in flow.rs (where resp, status and body are
computed) to emit a debug-level log/tracing event containing the HTTP status and
response body before returning the Err so callers/ops can triage (e.g., call
debug!(... or tracing::debug!(... ) with status = %status, body = %body) and
keep the existing Err(format!(...)) return unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05f5061f-1566-409a-864e-dfbaa9589a31
📒 Files selected for processing (7)
app/src/pages/onboarding/steps/__tests__/ApiKeysStep.test.tsxsrc/openhuman/inference/openai_oauth/flow.rssrc/openhuman/inference/openai_oauth/flow_tests.rssrc/openhuman/inference/ops_tests.rssrc/openhuman/inference/provider/factory_test.rssrc/openhuman/inference/schemas.rssrc/openhuman/inference/schemas_tests.rs
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Review — PR #2265: OpenAI OAuth (ChatGPT subscription) for inference
Solid feature work implementing PKCE OAuth for OpenAI via ChatGPT subscription. The module structure is clean, test coverage is strong (26 Rust tests, 7 frontend tests), and the additive design preserves existing API-key auth. Well done on the paste-callback UX compromise — pragmatic v1 that avoids the complexity of a localhost listener.
Change summary
| Area | Files | What changed |
|---|---|---|
| Rust core | openai_oauth/{config,flow,store,mod}.rs |
PKCE OAuth flow, token persistence, bearer resolution |
| Rust RPC | ops.rs, schemas.rs |
4 new controllers: start/complete/status/disconnect |
| Provider factory | factory.rs |
Early-return for openai slug → OAuth lookup |
| Frontend | ApiKeysStep.tsx |
"Sign in with ChatGPT" button + callback paste UI |
| Tests | flow_tests.rs, ops_tests.rs, factory_test.rs, schemas_tests.rs, ApiKeysStep.test.tsx |
Comprehensive coverage |
| Deps | Cargo.toml |
motosan-ai-oauth v0.2 |
Findings (non-CodeRabbit)
CodeRabbit already caught the critical block_on deadlock, missing HTTP timeout, silent upsert error, and RPC tracing gaps — I'm not repeating those. The findings below are project-specific issues they missed.
See inline comments for details (2 major, 1 minor).
| pub fn lookup_key_for_slug(slug: &str, config: &Config) -> anyhow::Result<String> { | ||
| if slug == "openai" { | ||
| return crate::openhuman::inference::openai_oauth::lookup_openai_bearer_token(config) | ||
| .map_err(|e| anyhow::anyhow!("[chat-factory] openai auth lookup failed: {e}")) |
There was a problem hiding this comment.
[major] This early return bypasses the standard lookup_key_for_slug path entirely for openai, and lookup_openai_bearer_token in store.rs reimplements the same API-key resolution logic (check auth_key_for_slug, fall back to bare slug). Two problems:
- Duplication — if the key lookup logic in
lookup_key_for_slugever changes (env var fallbacks, audit logging, metrics), the OpenAI path silently diverges. - Bypass risk — any provider-agnostic logic added below this block (e.g., rate limiting, key validation) will never run for OpenAI.
Suggestion: keep OAuth as a fallback rather than replacing the whole function. Let the existing lookup_key_for_slug run first; if it returns empty/default for openai, then try the OAuth path:
// At the END of lookup_key_for_slug, before the final return:
if slug == "openai" && result.is_empty() {
if let Ok(Some(token)) = openai_oauth::lookup_openai_bearer_token(config) {
return Ok(token);
}
}This removes the duplicated API-key logic from store.rs and keeps the standard path intact.
| tracing-appender = "0.2" | ||
| prometheus = { version = "0.14", default-features = false } | ||
| urlencoding = "2.1" | ||
| motosan-ai-oauth = { version = "0.2", features = ["codex"] } |
There was a problem hiding this comment.
[major] New external dependency motosan-ai-oauth handles security-sensitive material (OAuth tokens, PKCE verifiers, refresh tokens). Before merging:
- Verify crate provenance — author, download count, last publish date on crates.io
- Audit the
codexfeature specifically — confirm it only configures OAuth endpoints/client IDs and doesn't phone home or exfiltrate credentials - Check that
motosan_ai_oauth::refresh()(called instore.rs) sends the refresh token only to the configuredtoken_urland nowhere else
This is standard due diligence for any dependency that touches auth credentials. If the crate is maintained by a known/trusted party, note it in the PR description for reviewer confidence.
| } | ||
|
|
||
| pub(super) async fn exchange_authorization_code( | ||
| config: &motosan_ai_oauth::OAuthConfig, |
There was a problem hiding this comment.
[minor] Sending state in the token exchange request is non-standard OAuth2 — per RFC 6749 §4.1.3, the token request only requires grant_type, code, redirect_uri, and client_id. The state parameter is for the authorization request and callback validation only.
This is harmless with OpenAI's endpoint (they'll ignore it), but if the motosan-ai-oauth crate or this flow is ever reused with a stricter provider, it could cause a 400. Consider removing it unless OpenAI's docs explicitly require it.
# Conflicts: # src/openhuman/inference/provider/factory_test.rs
…en exchange - Build the reqwest client with a 20s timeout so a network stall on the OpenAI token endpoint cannot hang the OAuth completion flow indefinitely. - Stop sending `state` in the token request body. Per RFC 6749 §4.1.3 the token endpoint only needs grant_type/code/redirect_uri/code_verifier/ client_id; state belongs to the authorize redirect and is already validated server-side against the pending session before this call runs. - Log token-exchange transport and HTTP-error responses at warn level so the failure path is observable in production (addresses CodeRabbit comment on flow.rs:284 and graycyrus comment on flow.rs:260).
- Wrap the `Handle::block_on` of `motosan_ai_oauth::refresh` in `tokio::task::block_in_place` so the multi-thread runtime can move other tasks off this worker while the synchronous wait drives the refresh future. Calling `block_on` from inside an async caller on the same runtime thread is a known deadlock vector and `lookup_key_for_slug` is reached from async paths (e.g. `list_configured_models`). - Log upsert failures of the refreshed profile at warn level instead of silently dropping them; otherwise the fresh access token is lost on restart and the caller would have to refresh or re-auth again. - Slim `lookup_openai_bearer_token` to the OAuth path only; the factory-side `lookup_key_for_slug` now owns API-key resolution so the two paths no longer reimplement the same precedence logic.
Pair `inference_openai_oauth_{start,complete,status,disconnect}` with the
same `:start` / `:ok` / `:error` debug+error logging pattern the rest of
this RPC surface uses (see `inference_diagnostics` for the template), so
OAuth flow failures surface in production logs alongside the other
inference RPCs instead of being silently swallowed.
…_slug Previously the `openai` slug returned early into the OpenAI OAuth lookup, bypassing the standard new-style → legacy-bare-slug API-key resolution and any provider-agnostic logic (env overrides, audit logging, metrics, future key-validation hooks) that lives further down the function. It also meant the OAuth helper had to reimplement the same two-step API-key precedence. Run the standard lookup first; only fall back to the OAuth path when both the new-style and legacy API-key lookups return empty. The user-visible behavior stays the same (API key wins over OAuth when both are present), but the OAuth path can no longer silently skip provider-agnostic logic.
…hover `text-xs text-stone-500 dark:text-neutral-400 hover:text-stone-700 dark:hover:text-neutral-200 dark:text-neutral-200 …` had `dark:text-neutral-200` twice — the second copy comes after `dark:hover:text-neutral-200`, so under Tailwind's last-wins ordering the dark-mode hover variant could be defeated by the duplicate base class. Drop the redundant token (addresses CodeRabbit outside-diff comment on ApiKeysStep.tsx:253-259).
| tracing-appender = "0.2" | ||
| prometheus = { version = "0.14", default-features = false } | ||
| urlencoding = "2.1" | ||
| motosan-ai-oauth = { version = "0.2", features = ["codex"] } |
There was a problem hiding this comment.
@graycyrus tracking your [major] dependency-audit ask separately — this is a human-only item (I cannot verify crate provenance, audit the codex feature, or confirm refresh-token egress endpoints without operator access). Could you (or another maintainer) capture the audit notes in the PR description before merge so the rationale is preserved alongside the dependency add?
coderabbitai
Bot
added
the
feature
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/pages/onboarding/steps/ApiKeysStep.tsx (1)
69-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winState inconsistency if
openUrlfails.
setOauthAwaitingCallback(true)is set beforeawait openUrl(authUrl). IfopenUrlthrows, the catch block doesn't resetoauthAwaitingCallback, leaving the user seeing the callback URL input despite never reaching the auth page.Proposed fix: move state update after openUrl or reset in catch
} - setOauthAwaitingCallback(true); await openUrl(authUrl); + setOauthAwaitingCallback(true); } catch (err) { console.warn('[onboarding:api-keys] oauth start failed', err); setError('Could not start ChatGPT sign-in. Try again or use an API key.'); + setOauthAwaitingCallback(false); } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/onboarding/steps/ApiKeysStep.tsx` around lines 69 - 76, The code sets setOauthAwaitingCallback(true) before awaiting openUrl(authUrl), which leaves oauthAwaitingCallback true if openUrl throws; move the setOauthAwaitingCallback(true) call to immediately after the await openUrl(authUrl) so the flag is only set when navigation succeeds, or alternatively ensure the catch block resets it (call setOauthAwaitingCallback(false) inside the catch) so state remains consistent; update references around setOauthAwaitingCallback, openUrl(authUrl), and the catch/finally handling to implement one of these fixes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/pages/onboarding/steps/ApiKeysStep.tsx`:
- Around line 69-76: The code sets setOauthAwaitingCallback(true) before
awaiting openUrl(authUrl), which leaves oauthAwaitingCallback true if openUrl
throws; move the setOauthAwaitingCallback(true) call to immediately after the
await openUrl(authUrl) so the flag is only set when navigation succeeds, or
alternatively ensure the catch block resets it (call
setOauthAwaitingCallback(false) inside the catch) so state remains consistent;
update references around setOauthAwaitingCallback, openUrl(authUrl), and the
catch/finally handling to implement one of these fixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d1141dc9-bc3f-4d5b-906a-011dcff21ceb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.tomlapp/src/pages/onboarding/steps/ApiKeysStep.tsx
3 participants
Summary
src/openhuman/inference/openai_oauth/with token storage onprovider:openaiprofileoauth.openhuman.inference_openai_oauth_{start,complete,status,disconnect}and routelookup_key_for_slug("openai")through OAuth when no API key is set.ApiKeysStepwith “Sign in with ChatGPT”, browser authorize, and paste-callback completion flow.Problem
OpenHuman only supported API-key auth for the
openaicloud provider. Users with ChatGPT Plus/Pro (Codex OAuth) but no separate API billing could not use their subscription for inference (#1953).Solution
motosan-ai-oauthcodexprovider): PKCE authorize atauth.openai.com, loopback redirecthttp://127.0.0.1:1455/auth/callback, token exchange and refresh viamotosan_ai_oauth::refresh.Submission Checklist
diff-coverover normalized Vitest lcov + focusedcargo llvm-cov ... -- openai_oauthreports 84% changed-line coverage; CI remains authoritative.## Relateddocs/RELEASE-MANUAL-SMOKE.md) — N/A: not a release-cut doc changeCloses #NNNin the## RelatedsectionImpact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check(app Prettier + Rust fmt check passed in pre-push hook)pnpm typecheckpnpm debug unit ApiKeysStep(7 passed);CARGO_INCREMENTAL=0 CARGO_TARGET_DIR=$PWD/target cargo test openai_oauth --lib(26 passed)cargo fmt --manifest-path Cargo.toml --allapplied;cargo test openai_oauth --libgreen; workspace clippy run has noopenai_oauthdiagnostics but is blocked by unrelated pre-existing warnings-as-errors outside owned pathspnpm test:coveragepassed; localdiff-cover target/frontend-normalized.lcov target/openai-oauth.lcov --compare-branch=origin/main --fail-under=80passed at 84%.Validation Blocked
command:cargo clippy --manifest-path Cargo.toml --workspace --all-targets -- -D warningserror:fails with 535 pre-existing clippy warnings-as-errors across unrelated modules; searched the output and found noopenai_oauth/src/openhuman/inference/openai_oauthdiagnostics after the fixes.command:pnpm test:rusterror:fails in 40 unrelated memory tree tests because cloud embeddings require a backend session (No backend session for cloud embeddings); focusedopenai_oauthRust tests pass.command:git pushpre-pushpnpm rust:checkerror:isolated worktree lacks vendoredapp/src-tauri/vendor/tauri-cef, so Tauricargo check --manifest-path app/src-tauri/Cargo.tomlcannot load the vendoredtauridependency.impact:Push used--no-verifyonly for the isolated-worktree Tauri vendor blocker; CI remains authoritative for full Tauri checks.Behavior Changes
Parity Contract
provider:openaikey lookup paths unchanged when a key is present.inference/schemas.rs; factory delegates only for slugopenai.Duplicate / Superseded PR Handling
Summary by CodeRabbit