fix(onboarding): restore provider profiles on switch

[thanhtan1105]

Summary

• fix cloud → local provider switches so the router stays enabled and provider tiers are pinned correctly
• persist and restore per-provider model, router, and non-secret connection settings
• preserve Pilot, Smart Routing / llm_judge, disabled-mode, text-tier, and image-tier configuration across switches

Validation

• uv run pytest tests/test_onboarding/test_mutations.py tests/test_provider_bankr.py -q
• uv run ruff check src tests
• uv run mypy src/agentos --show-error-codes
• npm --prefix frontend run check
• uv build --wheel

Fixes #189
Refs #188

[keyKQ]

Solid fix for the core of #189 — switching cloud→local no longer disables the router, and I confirmed the secret hygiene holds: _provider_router_snapshot strips judge_api_key, literal api_key never reaches a profile, and "sk-literal-secret" does not appear anywhere in to_toml_dict(). Full suite is green on this branch (7310 passed, 27 skipped), plus ruff check and mypy (591 files).

My concerns are all about the shape of what gets persisted, not the fix itself. Three are worth resolving before merge; everything is reproduced below and inline.

1. Saved profiles freeze the shipped tier tables, so a model-ID bump stops reaching people.

to_toml_dict() (config.py:2035) deliberately drops agentos_router.tiers when they equal _router_tier_profile_defaults(tier_profile) — that elision is what lets a bump of the shipped model IDs propagate to existing installs. provider_profiles[*].agentos_router goes around 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'}

Bump those IDs next release and anyone who has ever switched providers gets the old ones back the moment they switch home. (~3.8 KB of frozen tier tables per profile in config.toml, too.)

2. The snapshot is the whole AgentOSRouterConfig, but most of it isn't provider-scoped.

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

Same for strategy, rollout_phase, auto_thinking, judge_short_circuit_*, kv_cache_anti_downgrade_enabled, complaint_upgrade_enabled, require_router_runtime. Only tier_profile / tiers / default_tier are genuinely per-provider.

3. restored_profile compares against a non-normalised old_provider — one-word fix, but it currently lets a mixed-case llm.provider skip reconcile entirely and leave the router pointing at another provider's models. Repro inline.

Rest is smaller: a phantom profile written for the never-configured default provider, an explicit model= being ignored by the restored router, a literal-api_key dead end, and a dead enabled = True line.

[keyKQ]

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).

[keyKQ]

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

[keyKQ]

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.

[keyKQ]

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.

[keyKQ]

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).

[keyKQ]

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.

[keyKQ]

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.

[keyKQ]

Merged — thanks @thanhtan1105. The cloud→local fix is right, and dropping and not current_profile from the is_local_provider branch is exactly what #189 needed.

I couldn't push to this branch (maintainer_can_modify is on, but the push is rejected), so the review follow-up is in #240 instead. It keeps your restore flow and narrows what gets persisted:

• ProviderProfileConfig.agentos_router → ProviderProfileConfig.router, holding only enabled / tier_profile / operator-authored tiers / judge target. The rest of AgentOSRouterConfig is install-wide, so snapshotting it per provider meant retuning e.g. pilot.safety_net_threshold while another provider was active got reverted on the next switch.
• Machine-written tier tables are no longer stored in a profile. to_toml_dict elides them for the live router so a shipped model-id bump reaches existing installs; profiles bypassed that and would have restored last release's ids.
• Reconcile now always runs after a restore, so an explicit model= re-pins a local provider's tiers instead of leaving them on the remembered model.
• restored_profile compares against the normalised provider, a stale same-provider profile no longer overrides live values on a single-field edit, and no profile is written for the never-configured default provider.

Full details and repros are in #240. Your commit is preserved in the history — this is a follow-up, not a replacement.