fix(settings): pre-populate model field from active routing config when editing existing provider#193
Conversation
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
interface/src/routes/Settings.tsx
Outdated
| staleTime: 10_000, | ||
| enabled: activeSection === "providers", | ||
| }); | ||
| const defaultAgentId = agentsData?.agents?.[0]?.id; |
There was a problem hiding this comment.
agentsData?.agents?.[0] assumes the default agent is always first in the list. That seems brittle; if there’s a stable default (e.g. main) or the API can expose the actual default, it’d be safer to select that explicitly.
| const defaultAgentId = agentsData?.agents?.[0]?.id; | |
| const defaultAgentId = | |
| agentsData?.agents?.find((agent) => agent.id === "main")?.id ?? agentsData?.agents?.[0]?.id; |
| // the model field with the current routing model so the | ||
| // user sees (and can adjust) what's actually active, | ||
| // rather than the hardcoded defaultModel placeholder. | ||
| const currentChannel = defaultAgentConfig?.routing?.channel; |
There was a problem hiding this comment.
One edge case: if the user clicks “Update” before defaultAgentConfig finishes loading, this still sets modelInput to defaultModel and won’t update when the query resolves. Might be worth re-computing modelInput when defaultAgentConfig arrives (while the dialog is open), or disabling the edit action until routing is available.
|
very helpful change |
WalkthroughClient-side Settings UI now pre-populates provider model inputs from agent routing configs and changes the Fireworks provider default model to Changes
Sequence Diagram(s)(No sequence diagrams generated.) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
interface/src/routes/Settings.tsx (2)
259-269:agents[0]default-agent selection is still brittle.
agentsData?.agents?.[0]?.idassumes the default agent is always first. If the list order is not guaranteed, this will silently select the wrong agent's config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 259 - 269, The current defaultAgentId uses a brittle index lookup (agentsData?.agents?.[0]?.id); change it to search the agents array for a designated default marker instead: use agentsData?.agents?.find(a => a.isDefault || a.default || a.is_default)?.id and fall back to the first agent id if no marker exists (i.e., agentsData?.agents?.[0]?.id). Update the location where defaultAgentId is computed (referencing agentsData, agents, defaultAgentId and the useQuery call to api.agents) so the code picks a true default agent rather than assuming array order.
534-543: Race condition: staledefaultModelshown ifdefaultAgentConfighasn't loaded at click time.If the user opens the edit dialog before the
agent-configquery resolves,currentChannelisundefined, thecurrentModelguard isnull, andmodelInputis set toprovider.defaultModel. BecausesetModelInputis called once at click time and the query result arriving later won't update an already-open dialog, the stale value persists until the dialog is closed and reopened.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 534 - 543, The current click-time logic sets modelInput using defaultAgentConfig which may be undefined when the agent-config query hasn't resolved, causing a stale provider.defaultModel to be shown; instead, initialize modelInput without relying solely on that synchronous value and move the population logic into a reactive effect: create a useEffect that depends on defaultAgentConfig, provider.id (and the dialog open state), compute currentChannel = defaultAgentConfig?.routing?.channel and currentModel as you do (using isConfigured(provider.id)), and call setModelInput(currentModel ?? provider.defaultModel ?? "") only when the edit dialog is open so the dialog updates if the query resolves after opening (also ensure you don’t clobber user edits by gating updates when the user has already modified modelInput).
🧹 Nitpick comments (2)
src/api/providers.rs (2)
962-979: OpenAI ChatGPT OAuth credential deletion does not emit aProviderSetupEvent.When credentials are saved (via
finalize_openai_oauth, line 416–419), aProvidersConfiguredevent is sent. But the deletion path here skips it, which may leave the system in a stale state (e.g., LLM manager still configured for a model whose credentials were just removed). Note: the regular provider deletion path (lines 1010–1017) also omits this, so this is a pre-existing gap—but worth flagging given that the OAuth flow explicitly relies on the event.♻️ Proposed fix: notify after credential removal
if let Some(mgr) = state.llm_manager.read().await.as_ref() { mgr.clear_openai_oauth_credentials().await; } + state + .provider_setup_tx + .try_send(crate::ProviderSetupEvent::ProvidersConfigured) + .ok(); return Ok(Json(ProviderUpdateResponse { success: true, message: "ChatGPT Plus OAuth credentials removed".into(), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/providers.rs` around lines 962 - 979, Deletion of OpenAI ChatGPT OAuth credentials does not emit the ProviderSetupEvent/ProvidersConfigured event, so after removing the credentials in the openai-chatgpt branch (the block that calls tokio::fs::remove_file and mgr.clear_openai_oauth_credentials()) send the same ProvidersConfigured/ProviderSetupEvent used in finalize_openai_oauth; specifically, after calling clear_openai_oauth_credentials() on state.llm_manager and before returning the ProviderUpdateResponse, publish/emit the ProvidersConfigured ProviderSetupEvent (matching the event type and payload used by finalize_openai_oauth) so listeners know the provider state changed.
98-123: API naming still references "Browser" OAuth while the implementation is now device-code flow.Types like
OpenAiOAuthBrowserStartRequest,OpenAiOAuthBrowserStartResponse, and thestart_openai_browser_oauth/openai_browser_oauth_statusfunction names reference "browser" OAuth, but the implementation now uses the device-code grant. Consider renaming in a follow-up to reduce confusion for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/providers.rs` around lines 98 - 123, The public types and handler names still reference "Browser" OAuth while the flow is actually device-code; rename OpenAiOAuthBrowserStartRequest, OpenAiOAuthBrowserStartResponse, OpenAiOAuthBrowserStatusRequest, OpenAiOAuthBrowserStatusResponse and the handler functions start_openai_browser_oauth and openai_browser_oauth_status to use "DeviceCode" (e.g., OpenAiOAuthDeviceCodeStartRequest/Response, OpenAiOAuthDeviceCodeStatusRequest/Response, start_openai_device_code_oauth, openai_device_code_oauth_status) and update any route registrations, imports, and usages to match the new names so the API surface and serialization remain consistent with the current device-code implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 270-275: The agent-config query (useQuery with queryKey
["agent-config", defaultAgentId]) is never invalidated after provider updates,
causing stale routing info; update both updateMutation.onSuccess and the success
branch of monitorOpenAiBrowserOAuth to call the React Query invalidation for the
agent-config prefix (e.g., queryClient.invalidateQueries(["agent-config"])) so
all variants (including ["agent-config", defaultAgentId]) are refreshed after
provider mutations or OAuth completion.
In `@src/api/providers.rs`:
- Around line 614-618: The current cast from device_code.expires_in (u64) to i64
can overflow; replace the direct `expires_in as i64` with a defensive conversion
using i64::try_from or clamping and use saturating_add to compute expires_at.
Concretely: convert device_code.expires_in (or the fallback
OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS) via i64::try_from(...).ok().or_else(...)
to a safe i64 value (or clamp to i64::MAX/remaining TTL), then compute
expires_at = now.saturating_add(safe_expires_in) so overflow cannot produce a
negative or wrapped timestamp; update the code around expires_in, expires_at,
and the device_code handling to use this safe conversion.
---
Duplicate comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 259-269: The current defaultAgentId uses a brittle index lookup
(agentsData?.agents?.[0]?.id); change it to search the agents array for a
designated default marker instead: use agentsData?.agents?.find(a => a.isDefault
|| a.default || a.is_default)?.id and fall back to the first agent id if no
marker exists (i.e., agentsData?.agents?.[0]?.id). Update the location where
defaultAgentId is computed (referencing agentsData, agents, defaultAgentId and
the useQuery call to api.agents) so the code picks a true default agent rather
than assuming array order.
- Around line 534-543: The current click-time logic sets modelInput using
defaultAgentConfig which may be undefined when the agent-config query hasn't
resolved, causing a stale provider.defaultModel to be shown; instead, initialize
modelInput without relying solely on that synchronous value and move the
population logic into a reactive effect: create a useEffect that depends on
defaultAgentConfig, provider.id (and the dialog open state), compute
currentChannel = defaultAgentConfig?.routing?.channel and currentModel as you do
(using isConfigured(provider.id)), and call setModelInput(currentModel ??
provider.defaultModel ?? "") only when the edit dialog is open so the dialog
updates if the query resolves after opening (also ensure you don’t clobber user
edits by gating updates when the user has already modified modelInput).
---
Nitpick comments:
In `@src/api/providers.rs`:
- Around line 962-979: Deletion of OpenAI ChatGPT OAuth credentials does not
emit the ProviderSetupEvent/ProvidersConfigured event, so after removing the
credentials in the openai-chatgpt branch (the block that calls
tokio::fs::remove_file and mgr.clear_openai_oauth_credentials()) send the same
ProvidersConfigured/ProviderSetupEvent used in finalize_openai_oauth;
specifically, after calling clear_openai_oauth_credentials() on
state.llm_manager and before returning the ProviderUpdateResponse, publish/emit
the ProvidersConfigured ProviderSetupEvent (matching the event type and payload
used by finalize_openai_oauth) so listeners know the provider state changed.
- Around line 98-123: The public types and handler names still reference
"Browser" OAuth while the flow is actually device-code; rename
OpenAiOAuthBrowserStartRequest, OpenAiOAuthBrowserStartResponse,
OpenAiOAuthBrowserStatusRequest, OpenAiOAuthBrowserStatusResponse and the
handler functions start_openai_browser_oauth and openai_browser_oauth_status to
use "DeviceCode" (e.g., OpenAiOAuthDeviceCodeStartRequest/Response,
OpenAiOAuthDeviceCodeStatusRequest/Response, start_openai_device_code_oauth,
openai_device_code_oauth_status) and update any route registrations, imports,
and usages to match the new names so the API surface and serialization remain
consistent with the current device-code implementation.
| const { data: defaultAgentConfig } = useQuery({ | ||
| queryKey: ["agent-config", defaultAgentId], | ||
| queryFn: () => api.agentConfig(defaultAgentId!), | ||
| staleTime: 10_000, | ||
| enabled: activeSection === "providers" && !!defaultAgentId, | ||
| }); |
There was a problem hiding this comment.
["agent-config"] is never invalidated after provider mutations — pre-populated model will be stale.
After updateMutation succeeds (or the ChatGPT OAuth flow completes), the backend updates the routing config, but neither updateMutation.onSuccess nor monitorOpenAiBrowserOAuth invalidates the ["agent-config", ...] cache. With staleTime: 10_000, the old routing model stays cached for up to 10 seconds. If the user saves a provider, then immediately clicks "Update" again, the dialog pre-populates the old channel — the opposite of what this PR intends.
🐛 Proposed fix — add agent-config invalidation in both mutation handlers
In updateMutation.onSuccess (around line 296):
setTimeout(() => {
queryClient.invalidateQueries({ queryKey: ["agents"] });
+ queryClient.invalidateQueries({ queryKey: ["agent-config"] });
queryClient.invalidateQueries({ queryKey: ["overview"] });
}, 3000);In monitorOpenAiBrowserOAuth success branch (around line 391):
setTimeout(() => {
queryClient.invalidateQueries({queryKey: ["agents"]});
+ queryClient.invalidateQueries({queryKey: ["agent-config"]});
queryClient.invalidateQueries({queryKey: ["overview"]});
}, 3000);Using the prefix key ["agent-config"] (no agent ID) invalidates all variants, so it works even before defaultAgentId is known.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/routes/Settings.tsx` around lines 270 - 275, The agent-config
query (useQuery with queryKey ["agent-config", defaultAgentId]) is never
invalidated after provider updates, causing stale routing info; update both
updateMutation.onSuccess and the success branch of monitorOpenAiBrowserOAuth to
call the React Query invalidation for the agent-config prefix (e.g.,
queryClient.invalidateQueries(["agent-config"])) so all variants (including
["agent-config", defaultAgentId]) are refreshed after provider mutations or
OAuth completion.
| let now = chrono::Utc::now().timestamp(); | ||
| let expires_in = device_code | ||
| .expires_in | ||
| .unwrap_or(OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS as u64); | ||
| let expires_at = now + expires_in as i64; |
There was a problem hiding this comment.
Potential integer overflow when casting expires_in from u64 to i64.
If the OAuth server returns an unexpectedly large expires_in value, expires_in as i64 wraps to a negative number, which would set expires_at in the past and immediately expire the session. Consider using i64::try_from or clamping.
🛡️ Suggested defensive cast
- let expires_at = now + expires_in as i64;
+ let expires_at = now + i64::try_from(expires_in).unwrap_or(OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let now = chrono::Utc::now().timestamp(); | |
| let expires_in = device_code | |
| .expires_in | |
| .unwrap_or(OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS as u64); | |
| let expires_at = now + expires_in as i64; | |
| let now = chrono::Utc::now().timestamp(); | |
| let expires_in = device_code | |
| .expires_in | |
| .unwrap_or(OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS as u64); | |
| let expires_at = now + i64::try_from(expires_in).unwrap_or(OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/api/providers.rs` around lines 614 - 618, The current cast from
device_code.expires_in (u64) to i64 can overflow; replace the direct `expires_in
as i64` with a defensive conversion using i64::try_from or clamping and use
saturating_add to compute expires_at. Concretely: convert device_code.expires_in
(or the fallback OPENAI_DEVICE_OAUTH_SESSION_TTL_SECS) via
i64::try_from(...).ok().or_else(...) to a safe i64 value (or clamp to
i64::MAX/remaining TTL), then compute expires_at =
now.saturating_add(safe_expires_in) so overflow cannot produce a negative or
wrapped timestamp; update the code around expires_in, expires_at, and the
device_code handling to use this safe conversion.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
interface/src/routes/Settings.tsx (1)
259-276:⚠️ Potential issue | 🟡 MinorInvalidate the new
agent-configquery after provider changes to avoid stale prefill.You introduced an
agent-configquery withstaleTime: 10_000, but it isn’t invalidated when providers are updated or after OAuth completes. That can leave the routing model stale for up to 10 seconds, defeating the new prefill behavior.🔧 Proposed fix — invalidate agent-config alongside agents/overview
setTimeout(() => { queryClient.invalidateQueries({ queryKey: ["agents"] }); + queryClient.invalidateQueries({ queryKey: ["agent-config"] }); queryClient.invalidateQueries({ queryKey: ["overview"] }); }, 3000);setTimeout(() => { queryClient.invalidateQueries({ queryKey: ["agents"] }); + queryClient.invalidateQueries({ queryKey: ["agent-config"] }); queryClient.invalidateQueries({ queryKey: ["overview"] }); }, 3000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 259 - 276, The new "agent-config" query can stay stale for up to 10s because it's not invalidated when providers change or OAuth flows complete; update the providers change and OAuth completion handlers to call the React Query invalidation for the agent-config key (e.g., invalidateQueries(["agent-config"]) or invalidateQueries(["agent-config", defaultAgentId]) via the queryClient) so that the defaultAgentConfig fetched by useQuery (queryKey ["agent-config", defaultAgentId] / queryFn api.agentConfig) is refreshed immediately after provider updates or OAuth completes, ensuring the routing model prefill is not stale; place the invalidation alongside the existing invalidation calls for agents/overview so it runs in the same update flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 278-289: The effect in useEffect that sets modelInput from
defaultAgentConfig can clobber a user's in-progress edits; modify the effect to
only call setModelInput when the input is still untouched (e.g., modelInput is
empty or equal to the default value). Concretely, in the useEffect that
currently checks defaultAgentConfig?.routing?.channel and calls
setModelInput(currentChannel), add a guard like if (!modelInput || modelInput
=== defaultModel) before setModelInput(currentChannel), and include modelInput
and defaultModel in the effect dependency array so the guard observes the latest
input state; keep editingProvider out if you still only want to trigger on
config arrival.
---
Duplicate comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 259-276: The new "agent-config" query can stay stale for up to 10s
because it's not invalidated when providers change or OAuth flows complete;
update the providers change and OAuth completion handlers to call the React
Query invalidation for the agent-config key (e.g.,
invalidateQueries(["agent-config"]) or invalidateQueries(["agent-config",
defaultAgentId]) via the queryClient) so that the defaultAgentConfig fetched by
useQuery (queryKey ["agent-config", defaultAgentId] / queryFn api.agentConfig)
is refreshed immediately after provider updates or OAuth completes, ensuring the
routing model prefill is not stale; place the invalidation alongside the
existing invalidation calls for agents/overview so it runs in the same update
flow.
| // If the routing config loads *after* the edit dialog is already open (race | ||
| // condition: user clicks edit before the agent-config query resolves), update | ||
| // the model field to show the active routing model instead of defaultModel. | ||
| useEffect(() => { | ||
| if (!editingProvider) return; | ||
| const currentChannel = defaultAgentConfig?.routing?.channel; | ||
| if (!currentChannel?.startsWith(`${editingProvider}/`)) return; | ||
| setModelInput(currentChannel); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [defaultAgentConfig]); | ||
| // Note: intentionally omitting editingProvider and modelInput from deps — we | ||
| // only want this to fire when the config data arrives, not on every keystroke. |
There was a problem hiding this comment.
Guard against overwriting user edits when the routing config arrives late.
If a user starts typing before defaultAgentConfig resolves, the effect will overwrite their input. Consider only replacing the model when it’s still the untouched default (or empty).
💡 Safer update to avoid clobbering user input
useEffect(() => {
if (!editingProvider) return;
const currentChannel = defaultAgentConfig?.routing?.channel;
if (!currentChannel?.startsWith(`${editingProvider}/`)) return;
- setModelInput(currentChannel);
+ const providerDefault =
+ PROVIDERS.find((p) => p.id === editingProvider)?.defaultModel ?? "";
+ setModelInput((prev) =>
+ prev === providerDefault || !prev ? currentChannel : prev
+ );
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [defaultAgentConfig]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@interface/src/routes/Settings.tsx` around lines 278 - 289, The effect in
useEffect that sets modelInput from defaultAgentConfig can clobber a user's
in-progress edits; modify the effect to only call setModelInput when the input
is still untouched (e.g., modelInput is empty or equal to the default value).
Concretely, in the useEffect that currently checks
defaultAgentConfig?.routing?.channel and calls setModelInput(currentChannel),
add a guard like if (!modelInput || modelInput === defaultModel) before
setModelInput(currentChannel), and include modelInput and defaultModel in the
effect dependency array so the guard observes the latest input state; keep
editingProvider out if you still only want to trigger on config arrival.
When editing an already-configured provider, the model field previously always showed the hardcoded defaultModel (e.g. llama-v3p3-70b-instruct for Fireworks) regardless of what model was actually active. This caused confusion: users saw a stale default instead of their real running model. Fix: fetch the default agent's routing config when on the providers tab. When opening the edit dialog for a configured provider, pre-populate the model field with routing.channel if it belongs to that provider. Falls back to defaultModel for new/unconfigured providers. Also updates the Fireworks defaultModel from the outdated llama-v3p3-70b-instruct to minimax-m2p5.
get_providers() only checked for anthropic_key in config.toml or the ANTHROPIC_API_KEY env var to determine if the Anthropic provider was configured. This caused the SetupBanner to show "No LLM provider configured" even when valid OAuth credentials existed in anthropic_oauth.json — which is the correct path after `spacebot auth login`. Mirror the existing openai_oauth_configured pattern for Anthropic: check credentials_path(&instance_dir).exists() and OR it into the anthropic bool in both config-exists and fallback branches.
- Use agents.find(a => a.id === 'main') instead of agents[0] to select the default agent more robustly (suggested by tembo bot) - Add useEffect to re-sync modelInput when defaultAgentConfig arrives after the edit dialog is already open, closing the race condition where clicking edit before the query resolves would leave the field showing defaultModel permanently (suggested by tembo bot)
johanjongsma
force-pushed
the
fix/provider-model-prepopulate
branch
from
4031ff1 to
8fd761c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
interface/src/routes/Settings.tsx (2)
287-295:⚠️ Potential issue | 🟡 MinorEffect still unconditionally overwrites in-progress user input.
The past review proposed guarding
setModelInputagainst overwriting when the user has already edited the field (e.g.prev === providerDefault || !prev). This remains unaddressed — if the user starts typing beforedefaultAgentConfigresolves, their edits are silently replaced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 287 - 295, The effect unconditionally calls setModelInput when defaultAgentConfig arrives, overwriting user edits; modify the useEffect (the one referencing editingProvider, defaultAgentConfig and calling setModelInput) to use the functional updater form setModelInput(prev => { if (prev !== undefined && prev !== '' && prev !== providerDefault) return prev; return currentChannel; }) so it only replaces the input when the previous value is empty or still equals providerDefault; keep the same dependency array behavior but perform this guarded update using currentChannel, editingProvider and providerDefault names to locate the code.
269-282:⚠️ Potential issue | 🟡 Minor
["agent-config"]is still never invalidated after mutations — pre-populated model stays stale.The past review flagged
updateMutation.onSuccess(lines 318–321) andmonitorOpenAiBrowserOAuth(lines 419–421). Both still omitqueryClient.invalidateQueries({ queryKey: ["agent-config"] }). Additionally,ConfigFileSection.updateMutation.onSuccess(lines ~1503–1506) now also has this gap — a raw config edit can change routing, yet the["agent-config"]cache is never busted there either.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/routes/Settings.tsx` around lines 269 - 282, The ["agent-config"] cache is never invalidated after mutations, causing stale pre-populated model data; update all mutation success handlers to call queryClient.invalidateQueries({ queryKey: ["agent-config"] }) so the agent config is refetched — specifically add this invalidation in updateMutation.onSuccess, in monitorOpenAiBrowserOAuth's success path, and in ConfigFileSection.updateMutation.onSuccess so any changes (including raw config edits that can change routing) refresh the cached ["agent-config"] data used by the useQuery that depends on defaultAgentId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@interface/src/routes/Settings.tsx`:
- Around line 287-295: The effect unconditionally calls setModelInput when
defaultAgentConfig arrives, overwriting user edits; modify the useEffect (the
one referencing editingProvider, defaultAgentConfig and calling setModelInput)
to use the functional updater form setModelInput(prev => { if (prev !==
undefined && prev !== '' && prev !== providerDefault) return prev; return
currentChannel; }) so it only replaces the input when the previous value is
empty or still equals providerDefault; keep the same dependency array behavior
but perform this guarded update using currentChannel, editingProvider and
providerDefault names to locate the code.
- Around line 269-282: The ["agent-config"] cache is never invalidated after
mutations, causing stale pre-populated model data; update all mutation success
handlers to call queryClient.invalidateQueries({ queryKey: ["agent-config"] })
so the agent config is refetched — specifically add this invalidation in
updateMutation.onSuccess, in monitorOpenAiBrowserOAuth's success path, and in
ConfigFileSection.updateMutation.onSuccess so any changes (including raw config
edits that can change routing) refresh the cached ["agent-config"] data used by
the useQuery that depends on defaultAgentId.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
interface/src/routes/Settings.tsxsrc/api/providers.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/providers.rs
2 participants
Problems fixed
1. Provider edit dialog shows stale defaultModel instead of active model
When editing an already-configured provider, the model field always showed the hardcoded
defaultModel(e.g.llama-v3p3-70b-instructfor Fireworks) regardless of what model was actually active in routing. Users saw a stale default instead of their real running model.Fix: Fetch the default agent's routing config when on the providers tab. Pre-populate the model field with
routing.channelwhen it belongs to the provider being edited. Falls back todefaultModelfor new/unconfigured providers.2. "No LLM provider configured" banner shows even with valid Anthropic OAuth
get_providers()only checked foranthropic_keyin config.toml orANTHROPIC_API_KEYenv var. Afterspacebot auth login, credentials are stored inanthropic_oauth.json— but the providers check ignored this file entirely, causing the SetupBanner to incorrectly warn that no provider was configured.Fix: Mirror the existing
openai_oauth_configuredpattern. Checkcrate::auth::credentials_path(&instance_dir).exists()and include it in the Anthropic provider bool.Changes
interface/src/routes/Settings.tsx: fetch agents + agent config on providers tab; pre-populate model from active routing; update Fireworks defaultModel tominimax-m2p5src/api/providers.rs: addanthropic_oauth_configuredcheck alongside API key detectionRepro for #2
spacebot auth login(Anthropic OAuth)anthropic_keyin config.toml orANTHROPIC_API_KEYenv var