Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions surfaces/cli/llm_auth/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
from dataclasses import dataclass
from typing import Literal

from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, SUPPORTED_PROVIDERS, ProviderOption
from surfaces.cli.wizard.config import (
PROVIDER_BY_VALUE,
SUPPORTED_PROVIDERS,
ProviderOption,
WizardCredentialKind,
)

AuthKind = Literal["api_key", "cli_subscription"]

Expand Down Expand Up @@ -70,7 +75,7 @@ def all_names(self) -> tuple[str, ...]:
def _api_key_profiles() -> tuple[ProviderAuthProfile, ...]:
profiles: list[ProviderAuthProfile] = []
for provider in SUPPORTED_PROVIDERS:
if provider.credential_kind != "api_key":
if provider.credential_kind != WizardCredentialKind.API_KEY:
continue
profiles.append(
ProviderAuthProfile(
Expand Down
6 changes: 3 additions & 3 deletions surfaces/cli/llm_auth/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
provider_for_profile,
resolve_auth_profile,
)
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption, WizardCredentialKind
from surfaces.cli.wizard.env_sync import sync_provider_env
from surfaces.cli.wizard.validation import validate_provider_credentials

Expand Down Expand Up @@ -88,7 +88,7 @@ def configure_api_key_provider(
) -> AuthSetupResult:
"""Validate and persist an API-key provider credential."""
provider = provider_for_profile(profile)
if provider.credential_kind != "api_key" or not provider.api_key_env:
if provider.credential_kind != WizardCredentialKind.API_KEY or not provider.api_key_env:
raise AuthSetupError(f"{provider.label} does not use an OpenSRE-managed API key.")

normalized_key = api_key.strip()
Expand Down Expand Up @@ -165,7 +165,7 @@ def configure_cli_subscription_provider(
) -> AuthSetupResult:
"""Configure a CLI-backed subscription provider such as ChatGPT/Codex or Claude Code."""
provider = provider_for_profile(profile)
if provider.credential_kind != "cli" or provider.adapter_factory is None:
if provider.credential_kind != WizardCredentialKind.CLI or provider.adapter_factory is None:
raise AuthSetupError(f"{provider.label} is not a CLI-backed subscription provider.")

adapter = provider.adapter_factory()
Expand Down
10 changes: 7 additions & 3 deletions surfaces/cli/wizard/_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
WARNING,
)
from surfaces.cli.llm_auth.persist import AuthSetupError, persist_api_key_secret
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption, WizardCredentialKind
from surfaces.cli.wizard.integration_health import IntegrationHealthResult
from surfaces.cli.wizard.probes import ProbeResult
from surfaces.cli.wizard.prompts import select as select_prompt
Expand Down Expand Up @@ -121,8 +121,12 @@ def _local_defaults() -> dict[str, str | bool | None]:
api_key_env = _string_value(
local.get("api_key_env"), api_key_provider.api_key_env if api_key_provider else ""
)
is_cli = bool(raw_provider_option and raw_provider_option.credential_kind == "cli")
is_host = bool(api_key_provider and api_key_provider.credential_kind == "host")
is_cli = bool(
raw_provider_option and raw_provider_option.credential_kind == WizardCredentialKind.CLI
)
is_host = bool(
api_key_provider and api_key_provider.credential_kind == WizardCredentialKind.HOST
)
is_oauth_backend = bool(raw_provider_value and raw_provider_value != provider_value)
raw_auth_method = local.get("auth_method")
auth_method = (
Expand Down
4 changes: 2 additions & 2 deletions surfaces/cli/wizard/env_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
write_env_lines,
)
from config.llm_auth.auth_method import LLM_AUTH_METHOD_ENV
from surfaces.cli.wizard.config import ProviderOption
from surfaces.cli.wizard.config import ProviderOption, WizardCredentialKind


def sync_reasoning_model_env(
Expand Down Expand Up @@ -162,7 +162,7 @@ def sync_provider_env(
# A ``host`` credential (e.g. the Ollama host) is non-secret runtime config
# that the wizard persists to ``.env`` — keep it as an active key so this
# sync does not strip it back out in the same wizard run.
if provider.credential_kind == "host" and provider.api_key_env:
if provider.credential_kind == WizardCredentialKind.HOST and provider.api_key_env:
active_non_secret.add(provider.api_key_env)
keys_to_remove -= active_non_secret

Expand Down
34 changes: 25 additions & 9 deletions surfaces/cli/wizard/flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,12 @@
from surfaces.cli.wizard.azure_openai import (
choose_provider_model,
)
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, SUPPORTED_PROVIDERS, ProviderOption
from surfaces.cli.wizard.config import (
PROVIDER_BY_VALUE,
SUPPORTED_PROVIDERS,
ProviderOption,
WizardCredentialKind,
)
from surfaces.cli.wizard.configurators.github import (
DEFAULT_GITHUB_MCP_MODE,
DEFAULT_GITHUB_MCP_URL,
Expand All @@ -71,6 +76,7 @@
from surfaces.cli.wizard.integration_health import IntegrationHealthResult
from surfaces.cli.wizard.llm_credential import (
CANCEL,
DEFERRED,
OK,
REPICK,
UNSAVED,
Expand Down Expand Up @@ -804,8 +810,8 @@ def run_wizard(_argv: list[str] | None = None) -> int:

if change_provider:
if auth_method == API_KEY_AUTH_METHOD and provider.credential_kind not in (
"cli",
"none",
WizardCredentialKind.CLI,
WizardCredentialKind.NONE,
):
credential_outcome, model = _prompt_validated_llm_credential(
provider,
Expand All @@ -818,7 +824,11 @@ def run_wizard(_argv: list[str] | None = None) -> int:
if credential_outcome == REPICK:
force_repick = True
continue
if credential_outcome == UNVERIFIED:
if credential_outcome == DEFERRED:
# No key to hand: finish onboarding with the provider chosen
# and nothing persisted, rather than ending the wizard.
credential_state = DEFERRED
Comment thread
greptile-apps[bot] marked this conversation as resolved.
elif credential_outcome == UNVERIFIED:
credential_state = UNVERIFIED
elif credential_outcome == UNSAVED:
credential_state = UNSAVED
Expand All @@ -836,16 +846,20 @@ def run_wizard(_argv: list[str] | None = None) -> int:
os.environ.update(azure_env)
else:
if auth_method == API_KEY_AUTH_METHOD and provider.credential_kind not in (
"cli",
"none",
WizardCredentialKind.CLI,
WizardCredentialKind.NONE,
):
has_api_key = bool(defaults["has_api_key"])
legacy_api_key = str(defaults["legacy_api_key"] or "").strip()
# A ``host`` credential (e.g. the Ollama host) is not a secret api key: never
# migrate a stale legacy ``api_key`` value into it — that would leak a
# secret-shaped value into .env and point the runtime at a bogus host. Fall
# through to the host prompt instead.
if not has_api_key and legacy_api_key and provider.credential_kind != "host":
if (
not has_api_key
and legacy_api_key
and provider.credential_kind != WizardCredentialKind.HOST
):
migration_outcome = _persist_llm_credential_with_recovery(
provider, legacy_api_key, session_env_sink=session_env_sink
)
Expand All @@ -869,7 +883,9 @@ def run_wizard(_argv: list[str] | None = None) -> int:
if credential_outcome == REPICK:
force_repick = True
continue
if credential_outcome == UNVERIFIED:
if credential_outcome == DEFERRED:
credential_state = DEFERRED
elif credential_outcome == UNVERIFIED:
credential_state = UNVERIFIED
elif credential_outcome == UNSAVED:
credential_state = UNSAVED
Expand All @@ -885,7 +901,7 @@ def run_wizard(_argv: list[str] | None = None) -> int:
provider_extra_env = azure_env
os.environ.update(azure_env)

if model_provider.credential_kind == "cli":
if model_provider.credential_kind == WizardCredentialKind.CLI:
cli_out = _run_cli_llm_onboarding(
model_provider,
display_label=(
Expand Down
33 changes: 23 additions & 10 deletions surfaces/cli/wizard/llm_credential.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
from surfaces.cli.wizard.azure_openai import (
choose_provider_model,
)
from surfaces.cli.wizard.config import PROJECT_ENV_PATH, ProviderOption
from surfaces.cli.wizard.config import PROJECT_ENV_PATH, ProviderOption, WizardCredentialKind
from surfaces.cli.wizard.endpoint_prompt import (
ensure_endpoint_settings as ensure_provider_endpoint_settings,
)
Expand All @@ -51,8 +51,8 @@
#: What became of the credential the wizard just collected. ``unsaved`` outranks
#: ``unverified``: a credential that never landed anywhere is the more urgent thing
#: to say on the summary screen.
CredentialState = Literal["ok", "unverified", "unsaved"]
CredentialOutcome = Literal["ok", "unverified", "unsaved", "repick", "cancel"]
CredentialState = Literal["ok", "unverified", "unsaved", "deferred"]
CredentialOutcome = Literal["ok", "unverified", "unsaved", "deferred", "repick", "cancel"]

# Recovery/outcome vocabulary. These are the byte-identical string values the wizard
# menus, ``_choose`` defaults, and ``run_wizard`` branches all resolve against; named
Expand All @@ -68,6 +68,10 @@
OK: Final = "ok"
UNSAVED: Final = "unsaved"
UNVERIFIED: Final = "unverified"
#: The user has no key to hand. Onboarding finishes; the key is added later with
#: ``opensre auth login <provider>`` or the provider env var. One value for both
#: the prompt outcome and the recorded state, as with ``unsaved``/``unverified``.
DEFERRED: Final = "deferred"

_LLM_CREDENTIAL_MAX_ATTEMPTS = 10 # mirrors _run_cli_llm_onboarding's retry budget

Expand Down Expand Up @@ -113,14 +117,16 @@ def _credential_line_for_saved_summary(
if provider.value == "openai":
return "OpenAI OAuth tokens (Codex CLI)"
return f"{_provider_choice_label(provider)} OAuth session"
if provider.credential_kind == "host":
if provider.credential_kind == WizardCredentialKind.HOST:
# A ``host`` credential (e.g. the Ollama host URL) is written to the project
# ``.env`` by ``_persist_llm_credential``, never the keyring — the summary must
# name .env, not the system keychain, in both the verified and unverified cases.
if credential_state == UNVERIFIED:
return "project .env (unverified)"
return "project .env"
if provider.credential_kind != "cli":
if provider.credential_kind != WizardCredentialKind.CLI:
if credential_state == DEFERRED:
return f"not set yet — run `opensre auth login {provider.value}` when you have a key"
if credential_state == UNSAVED:
return "not saved — re-enter next run"
if credential_state == UNVERIFIED:
Expand All @@ -135,7 +141,7 @@ def _credential_line_for_saved_summary(
def _persist_llm_credential(provider: ProviderOption, value: str) -> bool:
"""Persist one prompted credential where the runtime will actually read it.

``credential_kind == "host"`` values (e.g. the Ollama host URL) are plain
``credential_kind == WizardCredentialKind.HOST`` values (e.g. the Ollama host URL) are plain
runtime configuration, not secrets: the runtime resolves them from the
environment only, never the keyring, so they belong in the project ``.env``.
Everything else keeps the keyring path.
Expand All @@ -145,7 +151,7 @@ def _persist_llm_credential(provider: ProviderOption, value: str) -> bool:
(permission denied, read-only fs, full disk) fails soft exactly like a keyring
failure — it never propagates and crashes onboarding (#3591).
"""
if provider.credential_kind == "host":
if provider.credential_kind == WizardCredentialKind.HOST:
try:
sync_env_values({provider.api_key_env: value})
except OSError as exc:
Expand Down Expand Up @@ -249,7 +255,7 @@ def _persist_llm_credential_with_recovery(
why this is also correct at the legacy-key migration site, where there is no
prompt to return to.

Reachable for ``credential_kind == "host"`` only when the ``.env`` write fails:
Reachable for ``credential_kind == WizardCredentialKind.HOST`` only when the ``.env`` write fails:
``_persist_llm_credential`` returns ``False`` on an ``OSError`` write error, so the
host lands on the same recovery menu the keyring path uses (#3591). A successful
host write returns ``True`` and never reaches this menu.
Expand Down Expand Up @@ -353,15 +359,22 @@ def _prompt_validated_llm_credential(
for _attempt in range(_LLM_CREDENTIAL_MAX_ATTEMPTS):
try:
value = _prompt_value(
f"{credential_display} ({env_key})",
f"{credential_display} ({env_key}) — leave blank to set up later",
# Only a ``host`` credential may be pre-filled. _prompt_value returns the
# default on empty input, and a secret provider's credential_default is a
# placeholder (Azure's is an endpoint URL) — offering it would let a bare
# Enter persist a URL as the API key.
default=provider.credential_default if provider.credential_kind == "host" else "",
default=provider.credential_default
if provider.credential_kind == WizardCredentialKind.HOST
else "",
secret=provider.credential_secret,
# A blank answer is "I do not have this yet", not a mistake to
# re-prompt: this was the only step that could end onboarding.
allow_empty=provider.credential_kind != WizardCredentialKind.HOST,
back_on_cancel=True,
)
if not value:
return DEFERRED, model
except WizardBack: # must precede KeyboardInterrupt: WizardBack subclasses it
return REPICK, model
except KeyboardInterrupt:
Expand Down
Loading