chore: split ACP dry-run credential channels by role (#3717)

Address PR review: the dry-run lumped api_key_env_var, base_url_env_var, and
file_secrets[].secret_name into one "required" list, but they are not jointly
required — the API key and file-content creds are alternative auth paths (one
suffices) and the base URL is optional proxy routing. Replace
required_acp_secret_names with acp_api_key_secret_name / acp_base_url_secret_name
/ acp_file_secret_names so the editor/materialize can report set/missing
honestly instead of flagging a working api-key-only setup as incomplete.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Simon Rosenberg

openhands-sdk/openhands/sdk/profiles/resolver.py

@@ -94,8 +94,14 @@ class AgentProfileDiagnostics(BaseModel):
     resolved_mcp_servers: list[str] = Field(default_factory=list)
     dangling_mcp_server_refs: list[str] = Field(default_factory=list)
 
-    # ACP provider credential names the editor/materialize checks (ACP only).
-    required_acp_secret_names: list[str] = Field(default_factory=list)
+    # ACP provider credential channels the editor/materialize checks (ACP only).
+    # These are NOT jointly required: authentication needs the API key *or* one
+    # of the file-content credentials, and the base URL is optional proxy
+    # routing. Keeping them in separate fields lets the editor mark set/missing
+    # honestly instead of treating a working api-key-only setup as incomplete.
+    acp_api_key_secret_name: str | None = None
+    acp_base_url_secret_name: str | None = None
+    acp_file_secret_names: list[str] = Field(default_factory=list)
 
     # Redacted resolved settings, present iff ``valid``.
     resolved_settings: dict[str, Any] | None = None
@@ -177,22 +183,22 @@ def _api_key_set(llm: LLM) -> bool:
     return bool(value.strip()) and value != REDACTED_SECRET_VALUE
 
 
-def _required_acp_secret_names(acp_server: str) -> list[str]:
-    """Provider secret names derived from ``acp_server`` via ``ACP_PROVIDERS``.
+def _acp_credential_channels(
+    acp_server: str,
+) -> tuple[str | None, str | None, list[str]]:
+    """Provider credential channels for ``acp_server`` via ``ACP_PROVIDERS``.
 
-    The env-var key for the API credential and (optional) base URL, plus any
-    file-content credential secret names. Empty for ``'custom'`` servers.
+    Returns ``(api_key_env_var, base_url_env_var, file_secret_names)`` kept
+    separate by role: the API-key env var and the file-content credentials are
+    *alternative* auth mechanisms (one suffices), and the base URL is optional
+    proxy routing — not jointly required. All empty/``None`` for ``'custom'``
+    servers, whose creds the user manages directly.
     """
     info = get_acp_provider(acp_server)
     if info is None:
-        return []
-    names: list[str] = []
-    if info.api_key_env_var:
-        names.append(info.api_key_env_var)
-    if info.base_url_env_var:
-        names.append(info.base_url_env_var)
-    names.extend(spec.secret_name for spec in info.file_secrets)
-    return names
+        return None, None, []
+    file_names = [spec.secret_name for spec in info.file_secrets]
+    return info.api_key_env_var, info.base_url_env_var, file_names
 
 
 def _build_openhands_settings(
@@ -318,9 +324,11 @@ def resolve_agent_profile_dry_run(
                 f"LLM profile {profile.llm_profile_ref!r} not found"
             )
     else:
-        diagnostics.required_acp_secret_names = _required_acp_secret_names(
-            profile.acp_server
-        )
+        (
+            diagnostics.acp_api_key_secret_name,
+            diagnostics.acp_base_url_secret_name,
+            diagnostics.acp_file_secret_names,
+        ) = _acp_credential_channels(profile.acp_server)
 
     diagnostics.valid = not diagnostics.errors
     if diagnostics.valid:

tests/sdk/profiles/test_resolver.py

@@ -362,7 +362,7 @@ def test_dry_run_verdict_matches_real_resolve(
         )
 
 
-def test_dry_run_acp_reports_required_secret_names(
+def test_dry_run_acp_reports_credential_channels_by_role(
     llm_store: LLMProfileStore,
 ) -> None:
     profile = ACPAgentProfile(name="acp", acp_server="codex")
@@ -371,16 +371,15 @@ def test_dry_run_acp_reports_required_secret_names(
     )
     assert diag.agent_kind == "acp"
     assert diag.valid is True
-    # api_key + base_url env vars and the file-content secret name, from the registry.
-    assert diag.required_acp_secret_names == [
-        "OPENAI_API_KEY",
-        "OPENAI_BASE_URL",
-        "CODEX_AUTH_JSON",
-    ]
+    # The API key and the file-content credential are alternative auth paths;
+    # the base URL is optional proxy routing — each surfaced in its own field.
+    assert diag.acp_api_key_secret_name == "OPENAI_API_KEY"
+    assert diag.acp_base_url_secret_name == "OPENAI_BASE_URL"
+    assert diag.acp_file_secret_names == ["CODEX_AUTH_JSON"]
     assert diag.resolved_settings is not None
 
 
-def test_dry_run_acp_custom_server_has_no_required_secrets(
+def test_dry_run_acp_custom_server_has_no_credential_channels(
     llm_store: LLMProfileStore,
 ) -> None:
     profile = ACPAgentProfile(
@@ -389,7 +388,9 @@ def test_dry_run_acp_custom_server_has_no_required_secrets(
     diag = resolve_agent_profile_dry_run(
         profile, llm_store=llm_store, mcp_config=None, cipher=None
     )
-    assert diag.required_acp_secret_names == []
+    assert diag.acp_api_key_secret_name is None
+    assert diag.acp_base_url_secret_name is None
+    assert diag.acp_file_secret_names == []
 
 
 def test_dry_run_normalizes_settings_build_failure(