fix(acp): Only advertise OAuth when not already authenticated by jpshackelford · Pull Request #536 · OpenHands/OpenHands-CLI

jpshackelford · 2026-02-24T17:00:48Z

Summary

Fixes #535

When using the OpenHands CLI ACP server with JetBrains IDEs (IntelliJ, etc.), users see an "Authenticate with OpenHands Cloud" button in the AI Chat window even when they have already configured local LLM settings in ~/.openhands/agent_settings.json.

Root Cause

In openhands_cli/acp_impl/agent/base_agent.py, the initialize() method unconditionally returned OAuth as an available authentication method, regardless of whether authentication was actually needed.

Changes

Modified initialize() in base_agent.py to check _is_authenticated() before including auth methods in the response
When already authenticated (local agent has agent_settings.json, or cloud agent has valid API key), auth_methods is returned as an empty list
Updated and added tests to verify the conditional behavior

Expected Behavior After Fix

Local mode with existing settings: No auth button shown (auth_methods is empty)
Local mode without settings: Auth button shown (auth_methods contains OAuth)
Cloud mode with valid API key: No auth button shown
Cloud mode without API key: Auth button shown

Commands Run

make lint        # All checks passed
make test        # All 1257 tests passed

Testing

Added test_initialize_no_auth_methods_when_authenticated - verifies empty auth_methods when authenticated
Added test_initialize_auth_methods_when_not_authenticated - verifies OAuth is returned when not authenticated
Existing tests continue to pass

@jpshackelford can click here to continue refining the PR

🚀 Try this PR

uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@openhands/fix-acp-auth-advertise-535

When using local mode with agent_settings.json configured, the ACP server was always returning OAuth authentication methods in the InitializeResponse. This caused JetBrains IDEs to show an unnecessary 'Authenticate with OpenHands Cloud' button even when local LLM settings were already configured. Now, the initialize() method checks _is_authenticated() before including auth methods. If the user is already authenticated (local agent has agent_settings.json, or cloud agent has valid API key), auth_methods is returned as empty. Fixes #535 Co-authored-by: openhands <openhands@all-hands.dev>

github-actions · 2026-02-24T17:03:36Z

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands_cli/acp_impl/agent
base_agent.py	225	27	88%	129, 172, 213, 286–288, 353–356, 360–367, 392, 474–475, 478–479, 555, 625–627
local_agent.py	95	4	95%	114, 178, 188, 202
TOTAL	6505	905	86%

all-hands-bot

🟢 Good taste - This is how you fix UI problems: don't add handlers for things that shouldn't be shown, just don't show them.

Why this works:

Simple conditional eliminates a UI special case (showing auth when not needed)
Solves a real user problem, not a theoretical one
Tests cover both branches with real code paths (subclass over mocks - correct approach)
No added complexity, no breaking changes

Verdict: ✅ Merge-worthy. Clean, pragmatic fix with proper test coverage.

Key insight: Good code removes edge cases rather than handling them.

jpshackelford · 2026-02-24T19:50:03Z

I have put this back into draft because the PR doesn't properly consider the case of bedrock. I believe that one authentication mode for Bedrock is the boto3 is used to take authentication detail from an aws profile. Actually I think with Bedrock and LiteLLM there are three distinct modes of authentication:

AWS Profile
Combination of AWS_ACCESS_KEY_ID, AWS_ACCESS_KEY_ID, and AWS_REGION_NAME
New short lived / long lived tokens.

@OpenHands Please do look into the version of LiteLLM we are using in the CLI or transitively via OpenHands/software-agent-sdk and confirm or deny this claim.

openhands-ai · 2026-02-24T19:50:26Z

I'm on it! jpshackelford can track my progress at all-hands.dev

Add explicit None check before accessing auth_methods list in test to satisfy pyright's type narrowing. Also apply ruff formatting. Co-authored-by: openhands <openhands@all-hands.dev>

openhands-ai · 2026-02-24T19:52:49Z

Summary: LiteLLM Bedrock Authentication Investigation

You asked me to investigate the version of LiteLLM used in the CLI (and transitively via openhands-sdk) and confirm or deny your claim about Bedrock's 3 authentication modes.

Findings

Version Information:

LiteLLM version: 1.80.11 (locked in uv.lock)
openhands-sdk version: 1.11.5 (depends on LiteLLM)

✅ Your Claim is CONFIRMED

LiteLLM 1.80.11 explicitly supports all 3 authentication modes you mentioned, plus additional methods:

Mode	LiteLLM Parameters	Environment Variables
1. AWS Profile	`aws_profile_name`	`AWS_PROFILE`
2. Access Key + Secret + Region	`aws_access_key_id`, `aws_secret_access_key`, `aws_region_name`	`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_REGION_NAME`
3. Session Tokens	`aws_session_token`, `aws_role_name`, `aws_session_name`, `aws_web_identity_token`	(via STS/IAM Roles Anywhere)

Additionally, LiteLLM supports:

Bedrock API Keys: via api_key parameter or AWS_BEARER_TOKEN_BEDROCK environment variable
IAM instance profiles: boto3 auto-detects when running on EC2/ECS/Lambda

Implication for PR #536

Your concern about putting the PR back to draft is well-founded. The current _is_authenticated() implementation may only check for agent_settings.json or explicit API keys, but would miss users who are authenticated via:

AWS Profile in ~/.aws/credentials or ~/.aws/config
Environment variables (AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY)
IAM instance roles (EC2/ECS/Lambda)
Web identity tokens (EKS pod identity)

The PR needs to account for boto3's full credential chain to properly determine if a Bedrock user is "already authenticated."

No code changes were made - this was an information-gathering task to help you make an informed decision about the PR.

When no agent_settings.json exists, the local agent now checks for AWS credentials before showing the OAuth authentication button. This handles Bedrock users who authenticate via: - AWS environment variables (AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, AWS_PROFILE, AWS_DEFAULT_PROFILE) - AWS credential files (~/.aws/credentials, ~/.aws/config) This ensures Bedrock users don't see an unnecessary 'Authenticate with OpenHands Cloud' button when they have valid AWS credentials configured. Note: This only detects intent to use AWS authentication, it does not validate that the credentials are correct. Co-authored-by: openhands <openhands@all-hands.dev>

all-hands-bot

🟡 Acceptable - Core logic is solid, but AWS credential detection adds complexity for what appears to be a theoretical edge case.

Why the core fix has good taste:
Simple conditional eliminates UI clutter by not including auth_methods when not needed. This is exactly how you solve UI problems - don't show things that shouldn't be there, rather than adding handlers to hide them.

Concern:
The AWS credential detection logic (~90 lines of implementation + tests) solves a hypothetical problem: "users with AWS credentials but no agent_settings.json". If this isn't a proven user pain point, it's over-engineering.

See inline comments for specifics.

openhands_cli/acp_impl/agent/local_agent.py

tests/acp/test_agent.py

all-hands-bot

🟢 Good taste - This is how you fix UI problems: eliminate the special case rather than adding handlers for it.

Verdict: ✅ Merge-worthy

Core fix is elegant and solves a real production issue. AWS credential detection adds complexity but addresses confirmed customer needs with straightforward logic and proper test coverage.

openhands_cli/acp_impl/agent/base_agent.py

openhands_cli/acp_impl/agent/local_agent.py

tests/acp/test_agent.py

openhands_cli/acp_impl/agent/base_agent.py

openhands_cli/acp_impl/agent/local_agent.py

malhotra5

LGTM thank you!

jpshackelford · 2026-02-24T22:56:44Z

I am holding off merging until I get successful end-to-end test with IntelliJ IDEA. There appears to be another problem and it looks like it may be on the IntelliJ side.

jpshackelford · 2026-02-25T14:53:01Z

I am going to test at customer site with just #543 as this PR (#536) may be chasing a red herring. We'll see.

openhands-ai bot mentioned this pull request Feb 24, 2026

ACP server always advertises OAuth authentication even when local settings exist #535

Open

jpshackelford marked this pull request as ready for review February 24, 2026 19:39

Merge branch 'main' into openhands/fix-acp-auth-advertise-535

e6347c3

all-hands-bot reviewed Feb 24, 2026

View reviewed changes

jpshackelford marked this pull request as draft February 24, 2026 19:44

fix(tests): add None check for auth_methods to satisfy pyright

ff3e9fc

Add explicit None check before accessing auth_methods list in test to satisfy pyright's type narrowing. Also apply ruff formatting. Co-authored-by: openhands <openhands@all-hands.dev>

jpshackelford added review-this and removed review-this labels Feb 24, 2026

all-hands-bot reviewed Feb 24, 2026

View reviewed changes

openhands_cli/acp_impl/agent/local_agent.py Show resolved Hide resolved

openhands_cli/acp_impl/agent/local_agent.py Show resolved Hide resolved

tests/acp/test_agent.py Show resolved Hide resolved

jpshackelford marked this pull request as ready for review February 24, 2026 20:24

all-hands-bot approved these changes Feb 24, 2026

View reviewed changes

openhands_cli/acp_impl/agent/base_agent.py Show resolved Hide resolved

openhands_cli/acp_impl/agent/local_agent.py Show resolved Hide resolved

tests/acp/test_agent.py Show resolved Hide resolved

malhotra5 reviewed Feb 24, 2026

View reviewed changes

openhands_cli/acp_impl/agent/base_agent.py Show resolved Hide resolved

enyst reviewed Feb 24, 2026

View reviewed changes

openhands_cli/acp_impl/agent/local_agent.py Show resolved Hide resolved

malhotra5 approved these changes Feb 24, 2026

View reviewed changes

Merge branch 'main' into openhands/fix-acp-auth-advertise-535

f04fce5

Conversation

jpshackelford commented Feb 24, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Changes

Expected Behavior After Fix

Commands Run

Testing

🚀 Try this PR

Uh oh!

github-actions bot commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

jpshackelford commented Feb 24, 2026

Uh oh!

openhands-ai bot commented Feb 24, 2026

Uh oh!

openhands-ai bot commented Feb 24, 2026

Summary: LiteLLM Bedrock Authentication Investigation

Findings

✅ Your Claim is CONFIRMED

Implication for PR #536

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

malhotra5 left a comment

Choose a reason for hiding this comment

Uh oh!

jpshackelford commented Feb 24, 2026

Uh oh!

jpshackelford commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jpshackelford commented Feb 24, 2026 •

edited by github-actions bot

Loading

github-actions bot commented Feb 24, 2026 •

edited

Loading

jpshackelford commented Feb 25, 2026 •

edited

Loading