Add LLM subscription auth endpoints#3367
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 QA Report: PARTIAL
The new subscription API endpoints and SDK settings path worked up to the point that does not require a human OpenAI subscription login; completed OAuth/login-backed LLM use could not be verified without credentials.
Does this PR achieve its stated goal?
Partially. I verified with a real running agent-server that the PR adds the previously-missing OpenAI subscription endpoints: models/status/device-start/device-poll/logout all returned the expected HTTP statuses and safe response shapes, and the device-start call reached OpenAI and produced a real browser sign-in challenge. I also verified the SDK now preserves auth_type="subscription" / subscription_vendor="openai" in serialized settings and no longer silently creates a normal API-key LLM from that config. I could not verify the final connected status or an actual subscription-backed LLM call because that requires completing the OpenAI device-code flow with a ChatGPT subscription account.
| Phase | Result |
|---|---|
| Environment Setup | ✅ make build completed and the agent-server launched locally |
| CI Status | ⏳ Most checks are green; several build/QA/review checks were still in progress when checked |
| Functional Verification | 🟡 New endpoints/settings behavior verified; completed OAuth + real subscription LLM use not verified |
Functional Verification
Test 1: Subscription endpoints on the running agent-server
Step 1 — Reproduce / establish baseline without the PR:
Checked out main at 3d9fc105856acd1d8786b8ba76ea2f3dc8be2fc8, launched the real server:
OPENHANDS_SUPPRESS_BANNER=1 uv run python -m openhands.agent_server --host 127.0.0.1 --port 8010Then called existing and new API routes:
GET /api/llm/providers -> HTTP 200
GET /api/llm/subscription/openai/status -> {"detail":"Not Found"} HTTP 404
GET /api/llm/subscription/openai/models -> {"detail":"Not Found"} HTTP 404
This confirms the base branch served the existing LLM API but did not expose the subscription endpoints.
Step 2 — Apply the PR's changes:
Checked out add-llm-subscription-endpoints at c47c75573cf1d1b614458207ae34cc9387517867 and launched the real server on port 8011.
Step 3 — Re-run with the PR in place:
GET /api/llm/providers -> HTTP 200
GET /api/llm/subscription/openai/models -> HTTP 200
{"vendor":"openai","models":["gpt-5.1-codex-max","gpt-5.1-codex-mini","gpt-5.2","gpt-5.2-codex","gpt-5.3-codex"]}
GET /api/llm/subscription/openai/status -> HTTP 200
{"vendor":"openai","connected":false,"account_email":null,"expires_at":null}
POST /api/llm/subscription/openai/device/start -> HTTP 200
{"device_code":"<43-char opaque server token>","user_code":"<redacted live OpenAI user code>","verification_uri":"https://auth.openai.com/codex/device","verification_uri_complete":null,"expires_at":1779551456489,"interval_seconds":5}
POST /api/llm/subscription/openai/device/poll with the returned opaque token -> HTTP 200
{"vendor":"openai","connected":false,"account_email":null,"expires_at":null}
POST /api/llm/subscription/openai/device/poll with an invalid token -> HTTP 404
{"detail":"Subscription device login not found or expired"}
POST /api/llm/subscription/openai/logout -> HTTP 200
{"vendor":"openai","connected":false,"account_email":null,"expires_at":null}
This shows the PR adds functional HTTP endpoints, preserves the existing providers endpoint, returns an OpenAI device-code challenge through the real API path, keeps polling server-side via an opaque token, handles pending/invalid poll states, and returns safe status/logout payloads without OAuth access or refresh tokens.
Test 2: SDK subscription settings round-trip and runtime behavior
Step 1 — Reproduce / establish baseline without the PR:
On main, ran a small user-style SDK script that constructs an LLM with subscription fields, serializes OpenHandsAgentSettings, and calls create_agent():
LLM_CREATED {'model': 'gpt-5.2-codex', ...}
SETTINGS_DUMP {'model': 'gpt-5.2-codex', ...}
CREATE_AGENT_OK
The subscription fields were absent from the dump and the agent was created as a normal LLM configuration, confirming the base branch did not round-trip subscription auth settings.
Step 2 — Apply the PR's changes:
Checked out c47c75573cf1d1b614458207ae34cc9387517867 and ran the same script.
Step 3 — Re-run with the PR in place:
LLM_CREATED {'model': 'gpt-5.2-codex', 'auth_type': 'subscription', 'subscription_vendor': 'openai', ...}
SETTINGS_DUMP {'model': 'gpt-5.2-codex', 'auth_type': 'subscription', 'subscription_vendor': 'openai', ...}
CREATE_AGENT_ERROR ValueError OpenAI subscription login is required
This shows the PR preserves the subscription auth fields in serialized settings and changes runtime behavior from silently creating a regular API-key LLM to requiring an OpenAI subscription login before building the agent.
Unable to Verify
I could not complete the OpenAI browser device-code flow, verify connected: true, verify token refresh behavior, or make an actual subscription-backed LLM completion because this environment does not have a ChatGPT subscription account to authorize the live device challenge. Future QA would benefit from AGENTS.md guidance documenting whether a safe non-production OpenAI subscription test account or approved OAuth stub is available for end-to-end subscription login verification.
Issues Found
None from functional QA. The only gap is credential-bound verification of the completed OpenAI login and actual subscription LLM use.
This review was created by an AI agent (OpenHands) on behalf of the user.
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable, but I found a few correctness issues around subscription runtime resolution and device-login lifecycle. Inline comments have the actionable details.
Risk: 🟡 Medium — this touches auth/session state and LLM runtime wiring for long-running agents.
Verdict: COMMENT pending fixes.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26336675207
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable, but I found a few correctness issues around OAuth token handling and device-login lifecycle.
Risk: 🟡 Medium — this touches auth/session state and LLM runtime wiring for long-running agents.
Verdict: COMMENT pending fixes.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26337336844
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable, but I found a few correctness issues around subscription runtime state, device-login lifecycle, and profile switching.
Risk: 🟡 Medium — this touches auth/session state and long-running LLM/condenser behavior.
Verdict: COMMENT pending fixes.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26338423527
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟡 Taste Rating: Acceptable, but I found a few correctness issues around subscription runtime resolution and credential refresh.
Risk: 🟡 Medium — this touches auth/session state and async LLM runtime behavior.
Verdict: COMMENT pending fixes.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/software-agent-sdk/actions/runs/26411115492
| if model.startswith("openai/"): | ||
| model = model.removeprefix("openai/") | ||
|
|
||
| auth = OpenAISubscriptionAuth() |
There was a problem hiding this comment.
🟠 Important: This always creates a fresh OpenAISubscriptionAuth() with the default credential store, even when llm is already a runtime subscription LLM carrying a custom store or in-memory credentials from OpenAISubscriptionAuth(credential_store=...).create_llm(...). Since create_agent() and switch_llm() now call this helper, those valid LLMs can fail with OpenAI subscription login is required or silently switch accounts. Please make runtime subscription LLMs a no-op here, or carry their existing credential store/credentials into the new auth.
| label="API Key", | ||
| ), | ||
| ) | ||
| auth_type: Literal["api_key", "subscription"] = Field( |
There was a problem hiding this comment.
🟠 Important: Adding this public field makes LLM.load_from_env() accept LLM_AUTH_TYPE=subscription, but that loader still returns cls(**data) rather than rehydrating via the subscription runtime resolver. The result has auth_type='subscription' but _is_subscription=False, no Codex base URL, and no token refresh. Please route env-loaded subscription configs through the same create_subscription_llm_from_config() path used by JSON/settings.
| auth = OpenAISubscriptionAuth( | ||
| credential_store=self._subscription_credential_store | ||
| ) | ||
| credentials = auth.refresh_if_needed_sync() |
There was a problem hiding this comment.
🟠 Important: _atransport_call() also reaches this helper, so an expired subscription token performs a synchronous HTTP refresh on the event loop while the global LiteLLM modify_params lock is held. Long-running async conversations can stall unrelated requests during refresh. Please add an async subscription credential path for acompletion()/aresponses() or refresh before entering the synchronous transport section.
| """Return safe ChatGPT subscription connection state without tokens.""" | ||
| auth = _get_openai_subscription_auth() | ||
| try: | ||
| await auth.refresh_if_needed() |
There was a problem hiding this comment.
🟠 Important: refresh_if_needed() can write refreshed credentials, but this endpoint does it outside the device-login/logout lock. If a status request starts refreshing an expired old token while a device poll saves a newly completed login, the later refresh write can overwrite the new credentials. Please serialize all credential writes (refresh, save, logout) or add a compare-and-swap check in the credential store.
Summary
Tests
This PR was created by an AI agent (OpenHands) on behalf of the user.
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:9fd55ba-pythonRun
All tags pushed for this build
About Multi-Architecture Support
9fd55ba-python) is a multi-arch manifest supporting both amd64 and arm649fd55ba-python-amd64) are also available if needed