Skip to content

fix(onboarding): restore provider profiles on switch #239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
keyKQ merged 2 commits into from
Aug 7, 2026
Merged

fix(onboarding): restore provider profiles on switch #239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e307d7
fix(onboarding): restore provider profiles on switch
thanhtan1105 Aug 6, 2026
0cdcd01
chore(ci): trigger fresh pull request checks
thanhtan1105 Aug 7, 2026
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
13 changes: 13 additions & 0 deletions docs/configuration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,19 @@ English-optimized (Pilot)** (`pilot-v1`, the default), **Smart routing
only appears for the LLM-based strategy; the "Pilot safety net" field only
appears for the Pilot strategy.

#### Provider-switch profiles

When onboarding switches to another LLM provider, AgentOS saves a profile for
the provider being left and restores it when you return. A profile contains the
active model, router mode and settings (including text/image tiers, Smart
Routing judge model and endpoint, and Pilot settings), plus non-secret
connection settings such as `base_url`, `proxy`, `api_key_env`, and provider
routing preferences. Profiles are persisted in `config.toml`.

Literal `api_key` values and local `judge_api_key` values are not copied into a
profile. Prefer environment-variable references for credentials you need to
Comment on lines +427 to +437

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The credential caveat is documented, which I appreciate — but the actual failure is harder than "prefer env references" implies. A literal api_key makes the return trip fail, it doesn't just lose the key:

a = upsert_llm_provider(GatewayConfig(), provider_id="deepseek",
                        model="deepseek-chat", api_key="sk-literal-secret").config
b = upsert_llm_provider(a, provider_id="ollama", model="qwen3.5:9b").config
upsert_llm_provider(b, provider_id="deepseek")
# ValueError: provider 'deepseek' requires an api_key

From the setup UI that reads as a dead end: the profile restores model, base_url, proxy and routing, and then the mutation rejects the whole thing. Two things would help — say plainly in the docs that returning to a provider configured with a literal key requires re-entering it, and raise something the UI can act on ("provider 'deepseek' has a saved profile but no stored credential; re-enter the API key") rather than the generic message, which reads as "you never configured this".

Worth also stating here what the profile does not preserve on the router side, since it silently overwrites global router preferences on restore — see the ProviderProfileConfig comment.

survive a provider switch.

#### Upgrading from v4_phase3

Historical onboarding persisted `strategy = "v4_phase3"` explicitly in
Expand Down
19 changes: 19 additions & 0 deletions src/agentos/gateway/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,6 +1218,24 @@ def _resolve_tier_profile_defaults(cls, values: Any) -> Any:
AgentOSRouterConfig.model_rebuild()


class ProviderProfileConfig(BaseModel):
"""Restorable non-secret LLM and router settings for one provider.

Literal API credentials deliberately remain outside this snapshot. An
``api_key_env`` reference is safe to preserve; direct API keys continue to
use the active provider configuration and existing secret-handling paths.
"""

model: str
api_key_env: str = ""
base_url: str = ""
proxy: str = ""
max_tokens: int = 0
thinking: str | None = None
provider_routing: dict[str, str] = Field(default_factory=dict)
agentos_router: AgentOSRouterConfig
Comment on lines +1221 to +1236

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two structural issues with what this model persists.

(a) It freezes tier tables that to_toml_dict is careful not to freeze.

to_toml_dict at line 2035 pops agentos_router.tiers when they exactly match _router_tier_profile_defaults(tier_profile). That elision is load-bearing: it's how a bump of the shipped model IDs reaches installs that already have a config.toml. Nesting a full AgentOSRouterConfig inside provider_profiles bypasses it —

live router on deepseek -> 'tiers' key written?: False | tier_profile: deepseek
saved deepseek profile  -> 'tiers' key written?: True  | tier_profile: deepseek
  frozen: {'c0': 'deepseek-v4-flash', 'c1': 'deepseek-v4-flash',
           'c2': 'deepseek-v4-pro',  'c3': 'deepseek-v4-pro'}

So the next time the onboarding router models are bumped, anyone who has switched providers gets last release's IDs restored under them. Silently. Either run the same elision over each profile in to_toml_dict, or — cleaner — store tier_profile plus only operator-authored tier overrides, and re-derive the rest on restore.

(b) Most of AgentOSRouterConfig is a global preference, not a per-provider one.

on ollama, safety_net_threshold = 0.3
operator retunes on deepseek to = 0.9
after switching back to ollama  = 0.3   <- retune silently lost

pilot.safety_net_threshold has nothing to do with which provider is active, and neither do strategy, rollout_phase, auto_thinking, judge_input_max_chars, judge_short_circuit_enabled, judge_timeout_seconds, require_router_runtime, kv_cache_anti_downgrade_enabled, complaint_upgrade_enabled, upgrade_to_c3_compaction_enabled. An operator who tunes any of them while on provider B loses the tuning the moment they switch to A. That's a worse failure than the bug being fixed, because it's invisible — no warning, no diff, the value just reverts.

The fields that genuinely need per-provider memory are tier_profile, tiers, default_tier, and the judge target (judge_model / judge_provider / judge_base_url). I'd narrow ProviderProfileConfig.agentos_router to those rather than snapshotting the whole model.

(c) minor, but it bites operators: model and agentos_router are required, so a hand-edited or half-written profile makes the gateway refuse to start:

{'model': 'x'}          -> ValidationError: provider_profiles.ollama.agentos_router  Field required
{'agentos_router': {}}  -> ValidationError: provider_profiles.ollama.model           Field required

This is machine-written state living in a file operators do edit by hand. Strict-required fields on recoverable cache-like state trade a startup failure for a lost preference — worth defaults plus tolerant parsing (drop an unparseable profile, warn, carry on).



class AgentTokenSavingConfig(BaseSettings):
model_config = SettingsConfigDict(env_prefix="AGENTOS_AGENT_TOKEN_SAVING_")

Expand Down Expand Up @@ -1691,6 +1709,7 @@ class GatewayConfig(BaseSettings):
prompt: PromptConfig = Field(default_factory=PromptConfig)
memory: MemoryConfig = Field(default_factory=MemoryConfig)
agentos_router: AgentOSRouterConfig = Field(default_factory=AgentOSRouterConfig)
provider_profiles: dict[str, ProviderProfileConfig] = Field(default_factory=dict)
agent_token_saving: AgentTokenSavingConfig = Field(default_factory=AgentTokenSavingConfig)
compaction: CompactionLlmConfig = Field(default_factory=CompactionLlmConfig)
auxiliary: AuxiliaryConfig = Field(default_factory=AuxiliaryConfig)
Expand Down
133 changes: 103 additions & 30 deletions src/agentos/onboarding/mutations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
GatewayConfig,
LlmProviderConfig,
MemoryEmbeddingConfig,
ProviderProfileConfig,
_bankr_tiers,
_opencap_tiers,
_openrouter_tiers,
Expand Down Expand Up @@ -72,6 +73,13 @@ def _clone(cfg: GatewayConfig) -> GatewayConfig:
return new_cfg


def _provider_router_snapshot(router: AgentOSRouterConfig) -> AgentOSRouterConfig:
"""Copy router settings for a provider switch without duplicating secrets."""
payload = router.model_dump(mode="python")
payload.pop("judge_api_key", None)
return AgentOSRouterConfig(**payload)


def _clean_optional_str(value: str | None) -> str:
if value is None:
return ""
Expand Down Expand Up @@ -125,16 +133,21 @@ def _tiers_are_machine_written_defaults(
"""True when ``tiers`` are safe to rewrite (not operator-customised).

Two shapes count as machine-written:
* the shipped default tier sets (openrouter or bankr), matched exactly,
exactly as :meth:`GatewayConfig._default_agentos_router_profile_for_direct_provider`
* the shipped tier profiles, matched exactly, exactly as
:meth:`GatewayConfig._default_agentos_router_profile_for_direct_provider`
detects "not custom"; and
* tiers this reconcile already local-pinned — every entry's provider equals
the OLD llm provider AND every entry's model equals the OLD llm model
(a local→local switch, e.g. ollama→vllm, must re-pin these).
Anything else is treated as a custom, operator-authored tier set and left
untouched.
"""
if tiers in (_openrouter_tiers(), _bankr_tiers(), _opencap_tiers()):
if tiers in (
_openrouter_tiers(),
_bankr_tiers(),
_opencap_tiers(),
*(_router_tier_profile_defaults(profile) for profile in ROUTER_TIER_PROFILE_IDS),
):
return True
Comment on lines +145 to 151

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this rebuilds every shipped tier profile on each call, inside a tuple that's only used for an in test. _tiers_are_machine_written_defaults runs on every provider switch, so it's not hot, but the generator inside the tuple literal also makes the membership check quietly O(profiles) dict comparisons.

A module-level constant (or functools.cached helper) reads better and matches how _openrouter_tiers() / _bankr_tiers() are used elsewhere.

Also, now that the shipped tier profiles are covered generically, _openrouter_tiers(), _bankr_tiers() and _opencap_tiers() are likely redundant here if those three ids are in ROUTER_TIER_PROFILE_IDS — worth checking and collapsing to one source of truth.

old_provider = str(old_provider or "").strip().lower()
old_model = str(old_model or "").strip()
Expand Down Expand Up @@ -165,26 +178,24 @@ def _reconcile_router_profile_for_provider(
return []
if current_profile and str(current_profile).strip().lower() == provider_id:
return []
if is_local_provider(provider_id) and not current_profile:
if is_local_provider(provider_id):
# Local providers have no tier profile and build no per-tier client.
# When the current tiers are the untouched shipped defaults OR tiers a
# previous reconcile local-pinned, rewrite them to this provider+model so
# the persisted config is self-consistent (the runtime degrade guard then
# becomes a no-op).
if _tiers_are_machine_written_defaults(
cfg.agentos_router.tiers, old_provider, old_model
):
router_payload = cfg.agentos_router.model_dump(mode="python")
router_payload["tier_profile"] = None
router_payload = cfg.agentos_router.model_dump(mode="python")
router_payload["enabled"] = True

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is dead — the function already returned at line 177:

if not getattr(cfg.agentos_router, "enabled", True):
    return []

so enabled is always True by the time we get here. Verified: an operator who set mode="disabled" and then switches to a local provider keeps enabled = False, which is the correct behaviour and is not coming from this line.

That matters because the PR summary says this fixes switches "so the router stays enabled" — the actual fix is dropping and not current_profile from the is_local_provider condition, which is what stops a cloud→local switch from falling through to the enabled = False branch at line 211. Worth deleting the line so the next reader doesn't conclude this force-enables (and go looking for why disabled-mode survives).

router_payload["tier_profile"] = None
if _tiers_are_machine_written_defaults(cfg.agentos_router.tiers, old_provider, old_model):
router_payload["tiers"] = _local_provider_tiers(
cfg.agentos_router.tiers, provider_id, model
)
cfg.agentos_router = AgentOSRouterConfig(**router_payload)
return []
# Operator-customised tiers: leave the router exactly as the operator
# authored it (enabled + custom tiers). The runtime degrade guard pins
# any mismatched-provider tier to llm.model per turn, so custom local
# tiers stay safe without being clobbered here.
# Operator-customised tiers are preserved, but a local provider cannot
# retain a cloud tier profile. The runtime degrade guard pins any
# mismatched-provider tier to llm.model per turn, so custom local tiers
# stay safe without being clobbered here.
cfg.agentos_router = AgentOSRouterConfig(**router_payload)
return []
if (
not current_profile
Expand Down Expand Up @@ -360,7 +371,13 @@ def upsert_llm_provider(
raise ValueError(
f"provider {provider_id!r} is not runtime-supported and cannot be configured"
)
saved_profile = config.provider_profiles.get(provider_id)
active_provider = str(config.llm.provider or "").strip().lower()
model_clean = _clean_optional_str(model)
if not model_clean and saved_profile is not None:
model_clean = _clean_optional_str(saved_profile.model)
if not model_clean and active_provider == provider_id:
model_clean = _clean_optional_str(config.llm.model)
if not model_clean:
model_clean = _router_default_model_for_provider(
provider_id,
Expand All @@ -376,41 +393,97 @@ def upsert_llm_provider(
if api_key and api_key_env.strip():
raise ValueError("configure either api_key or api_key_env, not both")
effective_api_key_env = "" if api_key else api_key_env.strip()
if not api_key and not effective_api_key_env and config.llm.provider == provider_id:
effective_api_key_env = getattr(config.llm, "api_key_env", "").strip()
if not api_key and not effective_api_key_env:
if active_provider == provider_id:
effective_api_key_env = getattr(config.llm, "api_key_env", "").strip()
elif saved_profile is not None:
effective_api_key_env = saved_profile.api_key_env
if (
not effective_api_key
and spec.requires_api_key
and not api_key_env
and config.llm.provider == provider_id
and active_provider == provider_id
and config.llm.api_key
):
effective_api_key = config.llm.api_key
if spec.requires_api_key and not effective_api_key and not effective_api_key_env:
raise ValueError(f"provider {provider_id!r} requires an api_key")
effective_base_url = base_url or spec.default_base_url
saved_base_url = (
saved_profile.base_url
if saved_profile is not None
else (config.llm.base_url if active_provider == provider_id else "")
)
effective_base_url = base_url or saved_base_url or spec.default_base_url
if spec.requires_base_url and not effective_base_url:
raise ValueError(f"provider {provider_id!r} requires a base_url")
saved_proxy = (
saved_profile.proxy
if saved_profile is not None
else (config.llm.proxy if active_provider == provider_id else "")
)
effective_proxy = proxy or saved_proxy
saved_provider_routing = (
saved_profile.provider_routing
if saved_profile is not None
else (config.llm.provider_routing if active_provider == provider_id else {})
)
effective_provider_routing = (
dict(provider_routing) if provider_routing is not None else dict(saved_provider_routing)
)
saved_max_tokens = (
saved_profile.max_tokens
if saved_profile is not None
else (config.llm.max_tokens if active_provider == provider_id else 0)
)
saved_thinking = (
saved_profile.thinking
if saved_profile is not None
else (config.llm.thinking if active_provider == provider_id else None)
)

old_provider = str(config.llm.provider or "")
old_model = str(config.llm.model or "")
provider_profiles = {
str(provider).strip().lower(): profile.model_copy(deep=True)
for provider, profile in config.provider_profiles.items()
if str(provider).strip()
}
if old_provider and old_model:
provider_profiles[old_provider.strip().lower()] = ProviderProfileConfig(
model=old_model.strip(),
api_key_env=str(config.llm.api_key_env or "").strip(),
base_url=str(config.llm.base_url or "").strip(),
proxy=str(config.llm.proxy or "").strip(),
max_tokens=config.llm.max_tokens,
thinking=config.llm.thinking,
provider_routing=dict(config.llm.provider_routing),
agentos_router=_provider_router_snapshot(config.agentos_router),
)
Comment on lines +446 to +461

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A profile gets written for the default provider even when nobody ever configured it. GatewayConfig() ships provider="openrouter", model="openai/gpt-5.6-luna", so the very first upsert_llm_provider on a fresh install snapshots that as if it were a real setup:

upsert_llm_provider(GatewayConfig(), provider_id="ollama", model="qwen3.5:9b").config
# provider_profiles: ['openrouter']  ->  model 'openai/gpt-5.6-luna'

Right now it's invisible, because that value happens to equal _router_default_model_for_provider("openrouter", None). It stops being invisible the moment the shipped default moves: the operator later picks openrouter, gets the install-time model restored instead of the current recommended one, and _router_default_model_for_provider never runs. Combined with the frozen-tiers issue on ProviderProfileConfig, a fresh install ends up permanently pinned to whatever shipped the day it was installed.

Worth gating the snapshot on "this provider was actually configured" rather than "llm.provider and llm.model are non-empty" — they're non-empty by default. The field defaults are a usable signal, or config.config_path / the presence of credentials.

Also: nothing ever prunes this dict, so it grows one entry per provider ever touched and no reset path clears it.

restored_profile = provider_profiles.get(provider_id) if provider_id != old_provider else None

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug — old_provider here is the raw value (str(config.llm.provider or ""), line 444), not the normalised one. Every other comparison in this function uses active_provider (line 375, strip + lower). GatewayConfig doesn't normalise llm.provider on load, so a config with provider = "OpenCap" (or a stray trailing space) is reachable:

cfg = GatewayConfig(llm={"provider": "OpenCap", "model": "glm-5.2",
                        "api_key_env": "OPENCAP_API_KEY"})
res = upsert_llm_provider(cfg, provider_id="opencap", model="glm-5.3-new")
new llm.model  : glm-5.3-new
router tier c1 : gpt-5.6-luna     <- openrouter's default, on an opencap install
profiles       : ['opencap']

"opencap" != "OpenCap" is True, so restored_profile picks up the snapshot taken from the same provider two lines above, which sends us down the restored_profile is not None branch and skips _reconcile_router_profile_for_provider entirely. tier_profile never gets set to opencap and the tiers stay on whatever was there before.

Suggested change
restored_profile = provider_profiles.get(provider_id) if provider_id != old_provider else None
restored_profile = provider_profiles.get(provider_id) if provider_id != active_provider else None

new_cfg = _clone(config)
new_cfg.provider_profiles = provider_profiles
new_cfg.llm = LlmProviderConfig(
provider=provider_id,
model=model_clean,
api_key=effective_api_key,
api_key_env=effective_api_key_env,
base_url=effective_base_url,
proxy=proxy,
provider_routing=dict(provider_routing or {}),
)
reconcile_warnings = _reconcile_router_profile_for_provider(
new_cfg,
provider_id,
model=model_clean,
old_provider=old_provider,
old_model=old_model,
proxy=effective_proxy,
max_tokens=saved_max_tokens,
thinking=saved_thinking,
provider_routing=effective_provider_routing,
)
if restored_profile is not None:
new_cfg.agentos_router = restored_profile.agentos_router.model_copy(deep=True)
reconcile_warnings: list[str] = []
else:
reconcile_warnings = _reconcile_router_profile_for_provider(
new_cfg,
provider_id,
model=model_clean,
old_provider=old_provider,
old_model=old_model,
)
Comment on lines +476 to +486

@keyKQ keyKQ Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restoring a profile skips reconcile wholesale, which means an explicitly-passed model= is honoured for llm.model and ignored by the router:

a = upsert_llm_provider(GatewayConfig(), provider_id="ollama", model="qwen3.5:9b").config
b = upsert_llm_provider(a, provider_id="deepseek", model="deepseek-chat",
                        api_key_env="DEEPSEEK_API_KEY").config
back = upsert_llm_provider(b, provider_id="ollama", model="llama4:70b").config
llm.model      : llama4:70b
router tier c1 : {'provider': 'ollama', 'model': 'qwen3.5:9b', ...}
router tier c3 : {'provider': 'ollama', 'model': 'qwen3.5:9b', ...}

For a local provider that's a functional break, not just cosmetic: the whole point of the is_local_provider branch below is that local providers build no per-tier client, so every tier has to be pinned to llm.model. The runtime degrade guard won't rescue it either — it pins on provider mismatch, and the provider matches here. The router will keep asking ollama for a model the operator just replaced.

And from the setup UI this is exactly the flow you'd expect to hit: pick ollama again, pick a different model, submit.

Suggest treating an explicit model as authoritative — restore the profile, then still run the reconcile (or at least the local-pin) when model_clean differs from restored_profile.model. Note the restore path also hard-codes reconcile_warnings = [], so any warning the reconcile would have produced is dropped on the floor.

if api_key:
new_cfg.clear_runtime_secret("llm.api_key")

Expand All @@ -423,8 +496,8 @@ def upsert_llm_provider(
"explicit" if effective_api_key else ("env" if effective_api_key_env else "none")
),
"base_url": effective_base_url,
"proxy": proxy,
"provider_routing": dict(provider_routing or {}),
"proxy": effective_proxy,
"provider_routing": effective_provider_routing,
}
return MutationResult(
config=new_cfg,
Expand Down
Loading
Loading