fix(chat): model-dropdown switches actually stick - #73
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 31 seconds. β How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. π¦ How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. βΉοΈ Review infoβοΈ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: π Files selected for processing (4)
β¨ 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 |
360e50f to
8e1c657
Compare
|
@coderabbitai review |
β Actions performedReview triggered.
|
8e1c657 to
ccec7d6
Compare
|
@coderabbitai review |
β Actions performedReview triggered.
|
The ClawBox chat-panel model dropdown (`ChatPopup.tsx`) POSTs to `/setup-api/chat/model` when the user picks a model. That route previously did only two things: await exec(openclaw, ["config", "set", "agents.defaults.model.primary", target]); await restartGateway(); Both writes are correct, and new sessions opened after the restart inherit the chosen model. But they left the *currently open chat session's per-session override* untouched. That override lives in `<agents-dir>/<agent>/sessions/sessions.json` as: "providerOverride": "deepseek", "modelOverride": "deepseek-chat", "modelOverrideSource": "auto", "authProfileOverride": "deepseek:default", ... and it's what OpenClaw actually consults at chat time to decide which provider to dial. Because the source was `"auto"`, OpenClaw's own auto-picker happily rewrote it to whatever it preferred on every new message β usually the local provider when reachable β so the chat panel dropdown *appeared* to have no effect from the user's point of view. They would click "ClawBox AI", the pill would update, the next answer would still come from llama.cpp. Reproduced live on a Jetson: ran the CPU watch monitor on llama-server while the user picked ClawBox AI in the dropdown and sent messages β llama-server CPU stayed flat, the gateway's outbound TLS to openclawhardware.dev never opened, the dropdown was effectively decorative. Fix: - src/lib/openclaw-config.ts: new `applyModelOverrideToAllAgentSessions` helper that walks every sessions.json under the agents directory and rewrites the seven override fields to the given target, tagged with the given source (default `"manual"`). Atomic temp+rename writes so a crash mid-sweep can't corrupt sessions. Individual bad files are logged and skipped rather than aborting the whole sweep. Exported alongside a small `parseFullyQualifiedModel` helper for the `"<provider>/<modelId>"` format the rest of the codebase already uses. - src/app/setup-api/chat/model/route.ts: after the existing `agents.defaults.model.primary` write, call the new helper with `source: "manual"` so OpenClaw's auto-picker leaves the chosen override alone on subsequent messages. Sweep failures are logged but don't fail the request β worst case the default write still takes effect for newly-opened sessions. Also bump the per-call timeout on the underlying `openclaw config set` invocation from 10 s to 30 s: baseline CLI startup on Jetson Orin is 10-12 s (measured with three sequential idle runs), so the previous bound was guaranteed to trip and surface a spurious "command failed β¦ timed out after 10000ms" to the browser on every legitimate dropdown click. The helper alias is renamed `exec` β `runOpenclaw` inline to make it clear at the call site that we're invoking the `openclaw` CLI via `execFile` (no shell), not the `exec` family. Result: picking a model in the chat panel dropdown now changes the running session immediately and the change survives further messages. The auto-picker respects explicit user choices (since those are now tagged `manual`) and only fires on untouched sessions. The session-sweep helper is exported from `src/lib/openclaw-config.ts` so the pending Local-only-mode session-overrides fix (separate branch) can share the same implementation rather than duplicating the traversal and write logic.
The chat panel's `ChatMessage` type uses the same `role: 'system'` for both error banners (WebSocket dropped, model switch failed) and the "Switched chat to X." confirmation we emit after a successful dropdown pick. The shared style renders both in a red pill with red text, so a successful model switch looks like a failure in the stream. In the screenshot the user showed, "Switched chat to llamacpp/gemma4-e2b-it-q4_0." appeared directly above a real `Error:` banner in identical styling β no visual distinction. Changes in src/components/ChatPopup.tsx: - Add an optional `variant: 'success' | 'error'` to the `ChatMessage` interface. Defaults to `'error'` (no behaviour change for existing call sites: all the error paths keep their red styling). - Tag the `Switched chat to X.` message with `variant: 'success'`. - Update the inline message styles in the renderer to branch on `msg.variant` for `system`-role bubbles: emerald (`#22c55e` fg, `rgba(34,197,94,0.15)` bg) for success, keep the existing red (`#ef4444` fg, `rgba(239,68,68,0.15)` bg) for errors. Other system-role call sites β WS error banners, connection failures, "provider not configured" hints β all keep the default `'error'` variant and render red, so nothing else regresses.
β¦llyQualifiedModel PR ID-Robots#67's merge into beta brought runOpenclawConfigSet and the test mocks for it. This branch additionally adds two more openclaw-config helpers that the route now imports β extend the mock factory and the beforeEach with sensible defaults so the test file compiles and the route code paths that call them don't throw.
Applied code-simplifier to the branch diff before pushing for CodeRabbit review. Two substantive improvements surfaced: - openclaw-config.ts: collapsed the dual-name `SESSION_OVERRIDE_FIELDS` + `SESSION_OVERRIDE_FIELD_LIST` alias into a single direct export. The re-export was also throwing away the `as const` literal types by retyping to `readonly string[]`. - openclaw-config.ts: fixed a latent counter bug in applyModelOverrideToAllAgentSessions β `sessionsUpdated` was previously incremented inside the mutation loop before the atomic write, so a write failure would inflate the returned count. Now only accrues on successful write via a `touchedInFile` local. - ChatPopup.tsx: flattened the new triple-nested ternary for success-vs-error banner coloring by lifting `isSuccess`, `systemBg`, `systemColor` into locals in the message map callback. No behavior change aside from the counter-accuracy fix, which only matters when a sessions.json write fails mid-sweep.
ccec7d6 to
cdbee87
Compare
|
@coderabbitai review |
β Actions performedReview triggered.
|
Changing the primary AI provider in Settings (e.g. OpenAI β ClawBox AI) had three user-visible problems: 1. Chat kept responding as the OLD provider after the switch. The configure route updated agents.defaults.model.primary but left every existing session's per-session override pointing at the previous model. OpenClaw's resolver respected those sticky overrides on the next turn, so the chat stayed bound to the old provider until the user cleared the session β or refreshed the page and noticed it still answered wrong. 2. Chat 'froze' during the gateway restart. The chat popup only had skill-install-aware retry logic, so a provider-change gateway restart dropped the WS and the user saw no progress; only the bare retry loop slowly reconnected. 3. No affirmative 'switched to X' feedback β when reconnect finally landed, the chat looked identical to before. The dropdown already shows a green 'Switched chat to X' banner when switching models from within the chat pane (PR ID-Robots#73); provider changes via Settings deserved the same. Changes: - src/app/setup-api/ai-models/configure/route.ts: after 'agents.defaults.model.primary' is written, sweep per-session model overrides via applyModelOverrideToAllAgentSessions with source: 'user' so OpenClaw's resolver keeps the new primary bound across existing sessions. Mirrors the sweep in /setup-api/chat/model from PR ID-Robots#73. Only runs when this call actually set a new primary (skips local-only scopes). - src/components/ChatPopup.tsx: listens for the existing clawbox:primary-ai-configured event (dispatched by SettingsApp after provider configure succeeds) alongside clawbox-skill-installed. Reuses the reloadingSkill overlay + progress bar + quadrupled retry budget β makes the gateway restart visible instead of letting the chat look frozen. Banner label adapts ('Switching AI provider...' vs 'Reloading skills...') via a new reloadReason state; a matching reloadReasonRef escapes the frozen closure captured by the one-time WebSocket hello resolve callback. After reconnect on a provider change, the auto-'my skills were just updated' prompt is skipped (no skill changed, nothing to confirm) and a green 'Switched chat to <label>' banner is pushed using the refreshed chat/model state. - src/app/setup-api/chat/model/route.ts: PROVIDER_LABELS map was missing 'openai-codex', so labelForProvider fell back to the generic 'AI Provider' placeholder β which made the new banner read 'Switched chat to AI Provider' when switching to or from openai-codex. Added 'openai-codex: "OpenAI Codex"'. Post-fix /simplify pass converted the inline fetch().then().then() chain in ChatPopup's reload branch to async/await to match the existing refreshChatModelState pattern in the same file. Semantics unchanged.
#83) * fix(ai-provider): sweep sessions, show progress + green banner on switch Changing the primary AI provider in Settings (e.g. OpenAI β ClawBox AI) had three user-visible problems: 1. Chat kept responding as the OLD provider after the switch. The configure route updated agents.defaults.model.primary but left every existing session's per-session override pointing at the previous model. OpenClaw's resolver respected those sticky overrides on the next turn, so the chat stayed bound to the old provider until the user cleared the session β or refreshed the page and noticed it still answered wrong. 2. Chat 'froze' during the gateway restart. The chat popup only had skill-install-aware retry logic, so a provider-change gateway restart dropped the WS and the user saw no progress; only the bare retry loop slowly reconnected. 3. No affirmative 'switched to X' feedback β when reconnect finally landed, the chat looked identical to before. The dropdown already shows a green 'Switched chat to X' banner when switching models from within the chat pane (PR #73); provider changes via Settings deserved the same. Changes: - src/app/setup-api/ai-models/configure/route.ts: after 'agents.defaults.model.primary' is written, sweep per-session model overrides via applyModelOverrideToAllAgentSessions with source: 'user' so OpenClaw's resolver keeps the new primary bound across existing sessions. Mirrors the sweep in /setup-api/chat/model from PR #73. Only runs when this call actually set a new primary (skips local-only scopes). - src/components/ChatPopup.tsx: listens for the existing clawbox:primary-ai-configured event (dispatched by SettingsApp after provider configure succeeds) alongside clawbox-skill-installed. Reuses the reloadingSkill overlay + progress bar + quadrupled retry budget β makes the gateway restart visible instead of letting the chat look frozen. Banner label adapts ('Switching AI provider...' vs 'Reloading skills...') via a new reloadReason state; a matching reloadReasonRef escapes the frozen closure captured by the one-time WebSocket hello resolve callback. After reconnect on a provider change, the auto-'my skills were just updated' prompt is skipped (no skill changed, nothing to confirm) and a green 'Switched chat to <label>' banner is pushed using the refreshed chat/model state. - src/app/setup-api/chat/model/route.ts: PROVIDER_LABELS map was missing 'openai-codex', so labelForProvider fell back to the generic 'AI Provider' placeholder β which made the new banner read 'Switched chat to AI Provider' when switching to or from openai-codex. Added 'openai-codex: "OpenAI Codex"'. Post-fix /simplify pass converted the inline fetch().then().then() chain in ChatPopup's reload branch to async/await to match the existing refreshChatModelState pattern in the same file. Semantics unchanged. * fix(chat): address CodeRabbit #83 β preserve transcript + force hello on provider change Two Major findings on PR #83: 1. Provider switches were wiping the chat transcript. setMessages([]) ran unconditionally in the skillInstalledRef branch before the wasProviderChange check, so a Settings provider change cleared the user's visible history. Skill installs still want the reset (new tools may change the assistant's behavior), but provider changes don't β the conversation semantics are unchanged. Guarded the reset + greetedRef bump behind !wasProviderChange. 2. Provider-reload overlay could get stuck on fast gateway restarts. The configure route restarts the gateway before returning its response, and the primary-ai-configured event fires only after that response lands. If the WS retry loop has already reconnected by then, no future hello triggers the reload branch and the overlay stays up indefinitely. The provider handler now, after seeding state: - closes any currently-open WebSocket (so the resolve-branch fires on a fresh connect rather than relying on a stale hello that missed skillInstalledRef=true) - resets retryCountRef and calls connect() directly This makes the completion independent of any in-flight hello's timing relative to when the event is dispatched.
Summary
fix(chat): green confirmation pill on model switch, keep red for errors
The chat panel's
ChatMessagetype uses the samerole: 'system'for both error banners (WebSocket dropped, model switch failed) and
the "Switched chat to X." confirmation we emit after a successful
dropdown pick. The shared style renders both in a red pill with red
text, so a successful model switch looks like a failure in the
stream. In the screenshot the user showed, "Switched chat to
llamacpp/gemma4-e2b-it-q4_0." appeared directly above a real
Error:banner in identical styling β no visual distinction.Changes in src/components/ChatPopup.tsx:
variant: 'success' | 'error'to theChatMessageinterface. Defaults to
'error'(no behaviour change for existingcall sites: all the error paths keep their red styling).
Switched chat to X.message withvariant: 'success'.msg.variantforsystem-role bubbles: emerald (#22c55efg,rgba(34,197,94,0.15)bg) for success, keep the existing red(
#ef4444fg,rgba(239,68,68,0.15)bg) for errors.Other system-role call sites β WS error banners, connection
failures, "provider not configured" hints β all keep the default
'error'variant and render red, so nothing else regresses.fix(chat): model-dropdown switches actually stick to the active session
The ClawBox chat-panel model dropdown (
ChatPopup.tsx) POSTs to/setup-api/chat/modelwhen the user picks a model. That routepreviously did only two things:
await exec(openclaw, ["config", "set", "agents.defaults.model.primary", target]);
await restartGateway();
Both writes are correct, and new sessions opened after the restart
inherit the chosen model. But they left the currently open chat
session's per-session override untouched. That override lives in
<agents-dir>/<agent>/sessions/sessions.jsonas:"providerOverride": "deepseek",
"modelOverride": "deepseek-chat",
"modelOverrideSource": "auto",
"authProfileOverride": "deepseek:default",
...
and it's what OpenClaw actually consults at chat time to decide which
provider to dial. Because the source was
"auto", OpenClaw's ownauto-picker happily rewrote it to whatever it preferred on every new
message β usually the local provider when reachable β so the chat
Test plan