Skip to content

Commit 040253d

Browse files
authored
fix(onboarding): let a blank LLM key defer setup instead of blocking the wizard (#4994)
1 parent 56d283e commit 040253d

7 files changed

Lines changed: 203 additions & 33 deletions

File tree

surfaces/cli/llm_auth/providers.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55
from dataclasses import dataclass
66
from typing import Literal
77

8-
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, SUPPORTED_PROVIDERS, ProviderOption
8+
from surfaces.cli.wizard.config import (
9+
PROVIDER_BY_VALUE,
10+
SUPPORTED_PROVIDERS,
11+
ProviderOption,
12+
WizardCredentialKind,
13+
)
914

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

@@ -70,7 +75,7 @@ def all_names(self) -> tuple[str, ...]:
7075
def _api_key_profiles() -> tuple[ProviderAuthProfile, ...]:
7176
profiles: list[ProviderAuthProfile] = []
7277
for provider in SUPPORTED_PROVIDERS:
73-
if provider.credential_kind != "api_key":
78+
if provider.credential_kind != WizardCredentialKind.API_KEY:
7479
continue
7580
profiles.append(
7681
ProviderAuthProfile(

surfaces/cli/llm_auth/service.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
provider_for_profile,
3333
resolve_auth_profile,
3434
)
35-
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption
35+
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption, WizardCredentialKind
3636
from surfaces.cli.wizard.env_sync import sync_provider_env
3737
from surfaces.cli.wizard.validation import validate_provider_credentials
3838

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

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

171171
adapter = provider.adapter_factory()

surfaces/cli/wizard/_ui.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
WARNING,
3838
)
3939
from surfaces.cli.llm_auth.persist import AuthSetupError, persist_api_key_secret
40-
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption
40+
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, ProviderOption, WizardCredentialKind
4141
from surfaces.cli.wizard.integration_health import IntegrationHealthResult
4242
from surfaces.cli.wizard.probes import ProbeResult
4343
from surfaces.cli.wizard.prompts import select as select_prompt
@@ -121,8 +121,12 @@ def _local_defaults() -> dict[str, str | bool | None]:
121121
api_key_env = _string_value(
122122
local.get("api_key_env"), api_key_provider.api_key_env if api_key_provider else ""
123123
)
124-
is_cli = bool(raw_provider_option and raw_provider_option.credential_kind == "cli")
125-
is_host = bool(api_key_provider and api_key_provider.credential_kind == "host")
124+
is_cli = bool(
125+
raw_provider_option and raw_provider_option.credential_kind == WizardCredentialKind.CLI
126+
)
127+
is_host = bool(
128+
api_key_provider and api_key_provider.credential_kind == WizardCredentialKind.HOST
129+
)
126130
is_oauth_backend = bool(raw_provider_value and raw_provider_value != provider_value)
127131
raw_auth_method = local.get("auth_method")
128132
auth_method = (

surfaces/cli/wizard/env_sync.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
write_env_lines,
2222
)
2323
from config.llm_auth.auth_method import LLM_AUTH_METHOD_ENV
24-
from surfaces.cli.wizard.config import ProviderOption
24+
from surfaces.cli.wizard.config import ProviderOption, WizardCredentialKind
2525

2626

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

surfaces/cli/wizard/flow.py

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,12 @@
5454
from surfaces.cli.wizard.azure_openai import (
5555
choose_provider_model,
5656
)
57-
from surfaces.cli.wizard.config import PROVIDER_BY_VALUE, SUPPORTED_PROVIDERS, ProviderOption
57+
from surfaces.cli.wizard.config import (
58+
PROVIDER_BY_VALUE,
59+
SUPPORTED_PROVIDERS,
60+
ProviderOption,
61+
WizardCredentialKind,
62+
)
5863
from surfaces.cli.wizard.configurators.github import (
5964
DEFAULT_GITHUB_MCP_MODE,
6065
DEFAULT_GITHUB_MCP_URL,
@@ -71,6 +76,7 @@
7176
from surfaces.cli.wizard.integration_health import IntegrationHealthResult
7277
from surfaces.cli.wizard.llm_credential import (
7378
CANCEL,
79+
DEFERRED,
7480
OK,
7581
REPICK,
7682
UNSAVED,
@@ -804,8 +810,8 @@ def run_wizard(_argv: list[str] | None = None) -> int:
804810

805811
if change_provider:
806812
if auth_method == API_KEY_AUTH_METHOD and provider.credential_kind not in (
807-
"cli",
808-
"none",
813+
WizardCredentialKind.CLI,
814+
WizardCredentialKind.NONE,
809815
):
810816
credential_outcome, model = _prompt_validated_llm_credential(
811817
provider,
@@ -818,7 +824,11 @@ def run_wizard(_argv: list[str] | None = None) -> int:
818824
if credential_outcome == REPICK:
819825
force_repick = True
820826
continue
821-
if credential_outcome == UNVERIFIED:
827+
if credential_outcome == DEFERRED:
828+
# No key to hand: finish onboarding with the provider chosen
829+
# and nothing persisted, rather than ending the wizard.
830+
credential_state = DEFERRED
831+
elif credential_outcome == UNVERIFIED:
822832
credential_state = UNVERIFIED
823833
elif credential_outcome == UNSAVED:
824834
credential_state = UNSAVED
@@ -836,16 +846,20 @@ def run_wizard(_argv: list[str] | None = None) -> int:
836846
os.environ.update(azure_env)
837847
else:
838848
if auth_method == API_KEY_AUTH_METHOD and provider.credential_kind not in (
839-
"cli",
840-
"none",
849+
WizardCredentialKind.CLI,
850+
WizardCredentialKind.NONE,
841851
):
842852
has_api_key = bool(defaults["has_api_key"])
843853
legacy_api_key = str(defaults["legacy_api_key"] or "").strip()
844854
# A ``host`` credential (e.g. the Ollama host) is not a secret api key: never
845855
# migrate a stale legacy ``api_key`` value into it — that would leak a
846856
# secret-shaped value into .env and point the runtime at a bogus host. Fall
847857
# through to the host prompt instead.
848-
if not has_api_key and legacy_api_key and provider.credential_kind != "host":
858+
if (
859+
not has_api_key
860+
and legacy_api_key
861+
and provider.credential_kind != WizardCredentialKind.HOST
862+
):
849863
migration_outcome = _persist_llm_credential_with_recovery(
850864
provider, legacy_api_key, session_env_sink=session_env_sink
851865
)
@@ -869,7 +883,9 @@ def run_wizard(_argv: list[str] | None = None) -> int:
869883
if credential_outcome == REPICK:
870884
force_repick = True
871885
continue
872-
if credential_outcome == UNVERIFIED:
886+
if credential_outcome == DEFERRED:
887+
credential_state = DEFERRED
888+
elif credential_outcome == UNVERIFIED:
873889
credential_state = UNVERIFIED
874890
elif credential_outcome == UNSAVED:
875891
credential_state = UNSAVED
@@ -885,7 +901,7 @@ def run_wizard(_argv: list[str] | None = None) -> int:
885901
provider_extra_env = azure_env
886902
os.environ.update(azure_env)
887903

888-
if model_provider.credential_kind == "cli":
904+
if model_provider.credential_kind == WizardCredentialKind.CLI:
889905
cli_out = _run_cli_llm_onboarding(
890906
model_provider,
891907
display_label=(

surfaces/cli/wizard/llm_credential.py

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
from surfaces.cli.wizard.azure_openai import (
4242
choose_provider_model,
4343
)
44-
from surfaces.cli.wizard.config import PROJECT_ENV_PATH, ProviderOption
44+
from surfaces.cli.wizard.config import PROJECT_ENV_PATH, ProviderOption, WizardCredentialKind
4545
from surfaces.cli.wizard.endpoint_prompt import (
4646
ensure_endpoint_settings as ensure_provider_endpoint_settings,
4747
)
@@ -51,8 +51,8 @@
5151
#: What became of the credential the wizard just collected. ``unsaved`` outranks
5252
#: ``unverified``: a credential that never landed anywhere is the more urgent thing
5353
#: to say on the summary screen.
54-
CredentialState = Literal["ok", "unverified", "unsaved"]
55-
CredentialOutcome = Literal["ok", "unverified", "unsaved", "repick", "cancel"]
54+
CredentialState = Literal["ok", "unverified", "unsaved", "deferred"]
55+
CredentialOutcome = Literal["ok", "unverified", "unsaved", "deferred", "repick", "cancel"]
5656

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

7276
_LLM_CREDENTIAL_MAX_ATTEMPTS = 10 # mirrors _run_cli_llm_onboarding's retry budget
7377

@@ -113,14 +117,16 @@ def _credential_line_for_saved_summary(
113117
if provider.value == "openai":
114118
return "OpenAI OAuth tokens (Codex CLI)"
115119
return f"{_provider_choice_label(provider)} OAuth session"
116-
if provider.credential_kind == "host":
120+
if provider.credential_kind == WizardCredentialKind.HOST:
117121
# A ``host`` credential (e.g. the Ollama host URL) is written to the project
118122
# ``.env`` by ``_persist_llm_credential``, never the keyring — the summary must
119123
# name .env, not the system keychain, in both the verified and unverified cases.
120124
if credential_state == UNVERIFIED:
121125
return "project .env (unverified)"
122126
return "project .env"
123-
if provider.credential_kind != "cli":
127+
if provider.credential_kind != WizardCredentialKind.CLI:
128+
if credential_state == DEFERRED:
129+
return f"not set yet — run `opensre auth login {provider.value}` when you have a key"
124130
if credential_state == UNSAVED:
125131
return "not saved — re-enter next run"
126132
if credential_state == UNVERIFIED:
@@ -135,7 +141,7 @@ def _credential_line_for_saved_summary(
135141
def _persist_llm_credential(provider: ProviderOption, value: str) -> bool:
136142
"""Persist one prompted credential where the runtime will actually read it.
137143
138-
``credential_kind == "host"`` values (e.g. the Ollama host URL) are plain
144+
``credential_kind == WizardCredentialKind.HOST`` values (e.g. the Ollama host URL) are plain
139145
runtime configuration, not secrets: the runtime resolves them from the
140146
environment only, never the keyring, so they belong in the project ``.env``.
141147
Everything else keeps the keyring path.
@@ -145,7 +151,7 @@ def _persist_llm_credential(provider: ProviderOption, value: str) -> bool:
145151
(permission denied, read-only fs, full disk) fails soft exactly like a keyring
146152
failure — it never propagates and crashes onboarding (#3591).
147153
"""
148-
if provider.credential_kind == "host":
154+
if provider.credential_kind == WizardCredentialKind.HOST:
149155
try:
150156
sync_env_values({provider.api_key_env: value})
151157
except OSError as exc:
@@ -249,7 +255,7 @@ def _persist_llm_credential_with_recovery(
249255
why this is also correct at the legacy-key migration site, where there is no
250256
prompt to return to.
251257
252-
Reachable for ``credential_kind == "host"`` only when the ``.env`` write fails:
258+
Reachable for ``credential_kind == WizardCredentialKind.HOST`` only when the ``.env`` write fails:
253259
``_persist_llm_credential`` returns ``False`` on an ``OSError`` write error, so the
254260
host lands on the same recovery menu the keyring path uses (#3591). A successful
255261
host write returns ``True`` and never reaches this menu.
@@ -353,15 +359,22 @@ def _prompt_validated_llm_credential(
353359
for _attempt in range(_LLM_CREDENTIAL_MAX_ATTEMPTS):
354360
try:
355361
value = _prompt_value(
356-
f"{credential_display} ({env_key})",
362+
f"{credential_display} ({env_key}) — leave blank to set up later",
357363
# Only a ``host`` credential may be pre-filled. _prompt_value returns the
358364
# default on empty input, and a secret provider's credential_default is a
359365
# placeholder (Azure's is an endpoint URL) — offering it would let a bare
360366
# Enter persist a URL as the API key.
361-
default=provider.credential_default if provider.credential_kind == "host" else "",
367+
default=provider.credential_default
368+
if provider.credential_kind == WizardCredentialKind.HOST
369+
else "",
362370
secret=provider.credential_secret,
371+
# A blank answer is "I do not have this yet", not a mistake to
372+
# re-prompt: this was the only step that could end onboarding.
373+
allow_empty=provider.credential_kind != WizardCredentialKind.HOST,
363374
back_on_cancel=True,
364375
)
376+
if not value:
377+
return DEFERRED, model
365378
except WizardBack: # must precede KeyboardInterrupt: WizardBack subclasses it
366379
return REPICK, model
367380
except KeyboardInterrupt:

0 commit comments

Comments
 (0)