Repository navigation
fix(config): let a borrowed address follow the lender it came from - #512
Conversation
An EverOS role points a section at a provider: the model is written and that provider's key and address are copied in beside it. The address was copied only when there was one to copy, on the reasoning that a section with no lender address may be holding one the reader typed. But a section being pointed at a new lender is holding the OLD lender's address, and leaving the field out keeps it. Moving the memory role from an OpenRouter model to a DeepSeek one therefore left DeepSeek's key at OpenRouter's address, which the far end refuses -- and DeepSeek is not the corner case: every vendor LiteLLM routes carries no `default_api_base`, so the lend answers with no address for all of them. The lender answers about the address either way now, empty included, and an empty value reads where it lands the way an unset one does. Verified on a gateway: borrowing a vendor with an address writes it, borrowing one without clears what the section held. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the full diff and traced both callers: the settings RPC passes the explicit empty base_url into the EverOS writer, where it deletes the stale field, while CLI onboarding deliberately falls back to its curated provider URL. I also checked the lending and writer history, backward compatibility for lenders with configured/default addresses, test changes for weakening, and the applicable AGENTS.md/CLAUDE.md and CONTEXT-MAP.md/CONTEXT.md constraints.
Verification: uv run --all-packages pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py -x passed (523 tests; one non-failing asyncio cleanup warning). Ruff check/format, commit-message checks, source-language checks, and git diff --check passed. The first plain uv run pytest attempt could not collect the plugin-dependent suites because raven_everos was not installed; rerunning with all workspace packages supplied the plugin and completed successfully.
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The prior blocker is fixed on this revision. The lender now uses the same configured address, registry default, and canonical-name fallback precedence as the existing credential resolver. A direct reproduction of the OpenRouter-to-DeepSeek transition writes DeepSeek's key together with https://api.deepseek.com/v1, and the new RPC regression follows the value through settings.everosSet into everos.toml.
Reviewed the complete diff and the delta from the prior head, both callers, relevant resolver/writer history, backward compatibility, test integrity, and the applicable AGENTS.md/CLAUDE.md and CONTEXT-MAP.md/CONTEXT.md constraints. The remaining empty-address case is explicit for lenders with no usable address known to Raven and fails closed instead of retaining another lender's endpoint.
Verification: uv run --all-packages pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py -x passed (525 tests). Ruff check/format, commit-message checks, source-language checks, and git diff --check passed. The existing open review thread was started by another reviewer, so it was not resolved here.
The previous revision answered "" whenever the lender's section held no api_base, on the premise that an empty value reads the way an unset one does. It does not: the borrowed mapping enters the everos write through `clean.update(lent)`, which skips the empty-value filter the user's own fields pass, and the writer deletes a key whose value is "". So the incident path -- a role moved from an OpenRouter model to a DeepSeek one -- traded DeepSeek's key at OpenRouter's address for DeepSeek's key at no address, which the section's bare client refuses just as flatly. Resolve the address the way `resolve_provider_credentials` already does, through `_PROVIDER_BASE_URL_FALLBACK`: a registry entry naming no address is not a vendor without one. "" is now left only for a vendor no table here knows, where deleting is the deliberate answer -- what stands in the section was reached with somebody else's key. The unit test that pinned "" asserted the function's shape at its own boundary and never followed the value to the file. Replaced by one that names the address, plus an RPC regression that drives `settings_everos_set` and reads the written everos.toml. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
42ce2b9 to
e2b4a0d
Compare
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
This revision has the same source tree as the previously reviewed 42ce2b93eeb4; the range-diff changes only the latest commit's co-author trailer to the repository-required format. Tree identity and an empty git diff 42ce2b93eeb4 HEAD confirm that the previously verified behavior and 525-test result apply unchanged. scripts/check_commit_messages.py and git diff --check pass. The prior review thread has been resolved by its opener.
The fallback table closed deepseek, openai and openrouter; every other keyed vendor still answered "" for its address, which the everos writer reads as a delete. Measured end to end, a role pointed at a hand-typed gateway and refreshed from a groq key came back with no base_url at all -- and went on reporting itself configured, because `role_configured_in` and the LED beside it both ask `model and api_key` and neither reads the address. The loud failure I claimed for that branch happens only when something calls the role. groq and zai are in the tree already as `shown_api_base`; this copies them into the table this file reads. Not `display_api_base`, and not `usable_default_api_base`, which is empty for both on purpose so that neither sends its own base to the router -- the question here is the other one, what a bare OpenAI client must be told. What is left answering "" is two kinds, neither an oversight: a vendor on another protocol, and a deployment whose address only its operator knows. The comment says so, and says a vendor arriving there wants a table entry rather than that branch. Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The Groq/Z.ai blocker is fixed on this revision. Both providers now resolve through the existing bare-client fallback table; direct end-to-end checks through settings.everosSet persisted https://api.groq.com/openai/v1 and https://api.z.ai/api/paas/v4 respectively, while keeping the role configured. The unknown-address fixture now uses GPUStack, whose address is operator-defined and cannot have a global fallback.
Reviewed the full branch diff and the delta from e2b4a0d9992a, the RPC and onboarding callers, resolver/registry semantics, backward compatibility, test integrity, and the applicable repository/context constraints. uv run --all-packages pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py -x passed (526 tests). Ruff check/format, commit-message checks, source-language checks, and git diff --check passed. The remaining open blocker thread was started by another reviewer, so it was not resolved here.
) ## Summary An EverOS role points a section at a provider: the settings card writes the model and asks `lend_provider_credentials` for that provider's key and address to go beside it. The address was copied only when there was one to copy -- the reasoning being that a section with no lender address may be holding one the reader typed by hand. A section being pointed at a new lender is holding the OLD lender's address, though, and leaving the field out of the answer keeps it. Moving the memory role from an OpenRouter model to a DeepSeek one left DeepSeek's key at OpenRouter's address, which the far end refuses. DeepSeek is not a corner case here: every vendor LiteLLM routes carries no `default_api_base`, so the lend answers with no address for all of them. The lender answers about the address either way now, empty included. An empty value reads where it lands the way an unset one does: the EverOS writer sets the field, and the section stops carrying somebody else's endpoint. Found while running the settings page's acceptance case for the EverOS roles against a real gateway. The lend rule is from #414, not from the settings work that surfaced it. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - `uv run pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py tests/test_rpc_console.py -q` -- 641 passed on the branch this was written on; 191 on the two closest suites here. - The new case fails without the change: the lender answers `{"api_key": ...}` where the section needs `{"api_key": ..., "base_url": ""}`. - `uv run ruff check` and `ruff format --check` over both files -- clean. - Real gateway, temp `RAVEN_HOME`, EverOS plugin present: `settings.everosSet` with `borrow_from: "deepseek"` writes the model and DeepSeek's key and leaves no `base_url` behind; the same call with `borrow_from: "openrouter"` writes OpenRouter's address back. Before the change the first of those left `base_url = "https://openrouter.ai/api/v1"` under DeepSeek's key. - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed ## Risk - Two callers, both of which point a section at a provider: the EverOS card (`settings.everosSet`) and the CLI onboarding reuse step. For a lender that has an address, nothing changes. For one that has none, the borrowing section now stops carrying the previous lender's -- which is the fix. - A reader who typed an address by hand into an EverOS section and then borrowed from an addressless provider loses that typed value. That is the same event as the bug, read the other way; the section is being pointed somewhere else, and the card shows the resolved address. - Rollback: revert the commit. ## Related Issues N/A --------- Co-authored-by: gloryfromca <23442919+gloryfromca@users.noreply.github.com> Co-authored-by: Claude (claude-opus-5) <noreply@anthropic.com>
Summary
An EverOS role points a section at a provider: the settings card writes the
model and asks
lend_provider_credentialsfor that provider's key andaddress to go beside it. The address was copied only when there was one to
copy -- the reasoning being that a section with no lender address may be
holding one the reader typed by hand.
A section being pointed at a new lender is holding the OLD lender's address,
though, and leaving the field out of the answer keeps it. Moving the memory
role from an OpenRouter model to a DeepSeek one left DeepSeek's key at
OpenRouter's address, which the far end refuses. DeepSeek is not a corner
case here: every vendor LiteLLM routes carries no
default_api_base, so thelend answers with no address for all of them.
The lender answers about the address either way now, empty included. An empty
value reads where it lands the way an unset one does: the EverOS writer sets
the field, and the section stops carrying somebody else's endpoint.
Found while running the settings page's acceptance case for the EverOS roles
against a real gateway. The lend rule is from #414, not from the settings
work that surfaced it.
Type
Verification
uv run pytest tests/test_config_update_providers.py tests/test_rpc_settings.py tests/test_cli_onboard_commands.py tests/test_rpc_console.py -q-- 641 passedon the branch this was written on; 191 on the two closest suites here.
The new case fails without the change: the lender answers
{"api_key": ...}where the section needs
{"api_key": ..., "base_url": ""}.uv run ruff checkandruff format --checkover both files -- clean.Real gateway, temp
RAVEN_HOME, EverOS plugin present:settings.everosSetwith
borrow_from: "deepseek"writes the model and DeepSeek's key andleaves no
base_urlbehind; the same call withborrow_from: "openrouter"writes OpenRouter's address back. Before the change the first of those left
base_url = "https://openrouter.ai/api/v1"under DeepSeek's key.Relevant tests pass locally
Relevant lint / type checks pass locally
User-facing docs or screenshots are updated when needed
Risk
(
settings.everosSet) and the CLI onboarding reuse step. For a lender thathas an address, nothing changes. For one that has none, the borrowing
section now stops carrying the previous lender's -- which is the fix.
borrowed from an addressless provider loses that typed value. That is the
same event as the bug, read the other way; the section is being pointed
somewhere else, and the card shows the resolved address.
Related Issues
N/A