Skip to content

🐛 Agent LLM modal: stop wiping model/api_key on save, add validate-key flow#109

Open
MFA-X-AI wants to merge 2 commits into
mainfrom
fahreza/fix-llm-model-reset-on-save
Open

🐛 Agent LLM modal: stop wiping model/api_key on save, add validate-key flow#109
MFA-X-AI wants to merge 2 commits into
mainfrom
fahreza/fix-llm-model-reset-on-save

Conversation

@MFA-X-AI
Copy link
Copy Markdown
Member

@MFA-X-AI MFA-X-AI commented May 28, 2026

Noticed that editing an agent's LLM via Settings > Profile > Change silently destroys data. Typing a model name into the modal and clicking Apply writes model: null back to xpressclaw.yaml. On the second Apply, the stored api_key is also cleared. Restarting the agent doesn't bring either back — the value is gone from disk.

model.selection.error.mp4

The cause is a mismatch between the API surface and the data model. Since the per-agent LLM refactor (0b83c91), model lives inside AgentLlmConfig.model alongside provider / api_key / base_url, one struct per agent. The modal in ProfileTab.svelte, though, calls agents.updateConfig with model at the request top level AND an llm block that omits model. The PATCH handler in crates/xpressclaw-server/src/routes/agents.rs then runs two branches in sequence on the same request:

  1. Top-level model → write into llm.model (a legacy migration bridge from before the refactor).
  2. llm block → agent.llm = Some(req.llm), wholesale.

Step 2 always undoes step 1, so the model field gets dropped on every modal save. The same response returned has_api_key: bool instead of the value, so after each PATCH the modal's local llmApiKey state reset to ''; the next Apply then sent api_key: null, which the server faithfully wrote to disk.

This PR aligns the modal with the post-refactor data model:

  • applyModelConfig sends model inside the llm block, as a single unit.
  • handleSave (Save Changes button) stops sending top-level model. Model is the modal's responsibility, not the profile save bar's - they were both writing it.
  • The PATCH handler drops the now-dead top-level model branch entirely. UpdateAgentConfigRequest loses the field; the handler is one block that replaces agent.llm with req.llm, gated on a non-empty provider (empty provider still clears the binding so the router fails loudly instead of routing somewhere by accident).
  • The PATCH response now returns api_key (matching GET /api/setup/config) instead of has_api_key, so the modal's local state stays in sync across saves.

Legacy YAMLs with top-level agent.model continue to load - Config::migrate_legacy_fields() still moves the value into llm.model at file-load time, which is the right place for that migration. Only the API surface drops the duplicate path.

Plus: validate API key + fetch model list

While in the modal, this PR also lifts the first-time setup wizard's verify-and-fetch flow into the Profile → Change dialog so users can confirm credentials and pick a model from the upstream's list instead of typing it blind. A Validate button next to the API Key field (rendered for openai/anthropic, matching setup) calls the existing `POST /api/setup/validate-key` route, which already returns `{ valid, error?, models: [{ id }] }`. On success the Model field becomes a `` populated from that response; if the agent's stored model isn't in the list (relay-specific names, custom fine-tunes), it's kept as a "(custom)" option at the top of the dropdown so it's not silently dropped. Validation state resets when the modal opens and when the provider changes, so stale checkmarks don't carry over between sessions. Verified https://github.com/user-attachments/assets/0b4be573-8a79-4911-acb2-04ed926c3716 Agent route test suite passes 10/10, including a new `test_update_config_llm_block_round_trips_model_and_key` that locks in the contract — a single Apply round-trips model + api_key on disk, and the response carries `api_key` back so the modal stays accurate. Verified manually against a tailed `xpressclaw.yaml`: typing a model into the modal and clicking Apply now persists both `model` and `api_key`, re-Applying with no changes is a no-op, and selecting Provider → Default still clears the per-agent llm block. Validate-key flow tested live against the relay — button fires, models populate, custom-name preservation works.

MFA-X-AI added 2 commits May 28, 2026 13:18
The modal in ProfileTab.svelte sent `model` at the request top level
alongside an `llm` block that omitted model. The PATCH handler in
routes/agents.rs ran two branches in sequence on the same request:
a legacy migration branch wrote top-level model into llm.model, then
a full-replacement branch set agent.llm = req.llm — clobbering the
model the previous branch had just written. The response also
returned has_api_key: bool instead of the value, so after each PATCH
the modal's local llmApiKey reset to '' and the next Apply sent
api_key: null, wiping the stored key on disk.

Align the modal with the post-refactor data model (0b83c91): model
lives inside AgentLlmConfig, so send it inside the llm block as one
unit. Drop the now-dead top-level `model` field from
UpdateAgentConfigRequest and from the frontend's PATCH type. Save
Changes stops touching model — the modal's Apply path owns it.
PATCH response returns api_key (matching GET /api/setup/config) so
the modal's local state stays accurate across saves.

Legacy YAMLs with top-level agent.model still migrate into llm.model
via Config::migrate_legacy_fields() on load — that path is untouched.

New test_update_config_llm_block_round_trips_model_and_key locks in
the contract (10/10 agent route tests passing).
The Profile → Change modal had no way to verify credentials or discover
available models — users typed model names blind and only learned a key
or base_url was wrong by sending a chat message and watching it fail.

Lift the same flow the first-time setup wizard already uses into the
modal: a Validate button next to the API Key field (openai/anthropic
only, matching setup), a status line for the result, and a model select
populated from the upstream's /v1/models response on success. The
existing /api/setup/validate-key endpoint already returns { valid,
error?, models: [{id}] }, so no backend changes — just wire the modal
to it.

If the agent already had a model that isn't in the fetched list (e.g.
relay-specific tags), the stored value is preserved as a "(custom)"
option at the top of the dropdown so it's not silently dropped. Below
the validate-success threshold, the field stays a free-text input — the
old behavior — so ollama and unvalidated openai-compatible endpoints
still work.

Validation state (button status, model list) resets when the modal
opens and when the provider changes, so users don't see stale
checkmarks from a prior session.
@MFA-X-AI MFA-X-AI deployed to integration May 28, 2026 07:24 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant