fix: start the on-device model on demand and stop implying it is active - #367
Conversation
The local model could not be woken on a device whose agent config lives outside OpenClaw. The launcher decided whether to start by reading agents.defaults.model.primary from the OpenClaw config, which some editions never populate, so the gate was permanently closed: nothing spawned, and the caller then polled for twenty minutes against a server that was never launched. A chat with the on-device model selected hung for ten minutes and then failed. - pass the requested alias into the launcher; a request that arrived through the local-AI proxy is itself the authorization to start - for the boot-time path, also accept the edition-independent config-store record of "local AI is on". The existing config check stays first, so other editions are unchanged - report what a start attempt actually did, and fail immediately on a no-op instead of polling for a server nobody started - size the readiness wait for a wake (~18s measured) rather than reusing the multi-GB download budget - release the shared start slot as soon as an attempt settles, so one failure stops outliving its cause Settings also reported the on-device model as if it were in use whenever it was merely installed. Enabling it deliberately does not take over from the provider the customer picked, so the two states must read differently. The card now asks the active harness what it is really set to, names it, and only claims the local model when it is the selection. Its switch button now carries an explicit activation intent, so it actually switches instead of silently re-running the enable flow; the promote-on-first-setup policy is unchanged.
📝 WalkthroughWalkthroughThe PR adds explicit local-model activation across setup APIs and UI components. It resolves llama.cpp aliases from OpenClaw and ClawBox configuration, adds startup statuses, separates wake and provisioning timeouts, and adds coverage for activation, alias resolution, autostart, fail-fast behavior, and retries. ChangesLocal AI activation and runtime
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
🦀 ClawReviewPoked my eyestalks out for this one. Quick tour: Three related bugs in the on-device llama.cpp flow fixed together: the model could never start on Hermes devices because the start gate read from an OpenClaw config that Hermes never writes; a failed start went undetected and turned every subsequent inference request into a 20-minute hang; and Settings displayed the local model as active whenever it was installed, even if the harness was pointed at a cloud provider. The fix threads an explicit alias through the launcher, distinguishes "installed" from "selected" at the UI and API layers, and replaces the silent no-op return with a typed status so callers can fail fast. At a glance
Good to know
— ClawReview 🦀. I set the scene; CodeRabbit reviews the code; you decide. Conventions: docs. |
github-actions
Bot
added
area: gateway
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/setup-api/ai-models/configure/route.ts (1)
978-994: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn an error when Hermes activation fails.
When
shouldPromoteLocalToPrimaryis true,applyLocalAiToHermesmust update Hermesmodel.providerandmodel.default. The catch block logs a failure and the route later returns success. The user can select Gemma, receive HTTP 200, and remain on the old Hermes provider.Keep this registration non-fatal for fallback-only setup. Return a failure when
makeDefaultis required.Proposed fix
} catch (err) { - // Non-fatal: the local model is configured and running either way. console.error("[ai-models/configure] Hermes local provider registration failed:", err); + if (shouldPromoteLocalToPrimary) { + return NextResponse.json( + { error: "Local AI was configured, but Hermes could not activate it. Please try again." }, + { status: 502 }, + ); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/setup-api/ai-models/configure/route.ts` around lines 978 - 994, The catch around applyLocalAiToHermes must propagate a failure when shouldPromoteLocalToPrimary (and therefore makeDefault) is true, so the route does not return success after Hermes activation fails. Preserve the existing non-fatal logging and successful fallback-only behavior when promotion is not required, using the surrounding route’s established error-response pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/SettingsApp.tsx`:
- Around line 1046-1048: Update the localAiIsActive condition in SettingsApp so
it compares the normalized active model against localAiStatus.model in addition
to the existing configured and provider checks; keep the Hermes provider-slot
matching behavior while ensuring a different local model does not mark Local AI
as selected.
- Around line 2414-2421: Update the local-model activation onConfigured callback
in SettingsApp so it refreshes both localAiStatus and aiProvider after success.
Fetch /setup-api/ai-models/status in that callback, derive the current provider
from the response, and update aiProvider so the panel immediately reflects Gemma
4 activation without requiring a remount.
In `@src/instrumentation-node.ts`:
- Around line 188-191: Update the existing-process branch in the instrumentation
startup flow to verify that the live PID belongs to the requested alias before
returning "started". Persist and read the launch alias alongside the PID, or
inspect the active process arguments, and only return "started" when it matches
alias; otherwise continue with the appropriate new-launch handling.
In `@src/lib/local-ai-runtime.ts`:
- Around line 127-129: Use provisioning.installed, rather than
provisioning.modelAvailable, when selecting the wake timeout in the
local-ai-runtime readiness budget. In src/lib/local-ai-runtime.ts lines 127-129,
preserve the startup timeout unless installed is true; in
src/tests/unit/local-ai-runtime-failfast.test.ts lines 95-105, set installed:
true on the wake fixture and add coverage verifying a missing binary uses the
startup budget.
---
Outside diff comments:
In `@src/app/setup-api/ai-models/configure/route.ts`:
- Around line 978-994: The catch around applyLocalAiToHermes must propagate a
failure when shouldPromoteLocalToPrimary (and therefore makeDefault) is true, so
the route does not return success after Hermes activation fails. Preserve the
existing non-fatal logging and successful fallback-only behavior when promotion
is not required, using the surrounding route’s established error-response
pattern.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 17da9276-29cd-4d42-ab11-3e96ec55f15d
📒 Files selected for processing (15)
src/app/setup-api/ai-models/configure/route.tssrc/app/setup-api/ai-models/status/route.tssrc/app/setup-api/llamacpp/install/route.tssrc/components/AIModelsStep.tsxsrc/components/LlamaCppModelPanel.tsxsrc/components/SettingsApp.tsxsrc/hooks/useLlamaCppModels.tssrc/instrumentation-node.tssrc/lib/llamacpp-server.tssrc/lib/local-ai-runtime.tssrc/tests/components/llamacpp-model-panel-activate.test.tsxsrc/tests/routes/ai-models/configure-hermes.test.tssrc/tests/unit/llamacpp-configured-alias.test.tssrc/tests/unit/local-ai-autostart.test.tssrc/tests/unit/local-ai-runtime-failfast.test.ts
| const localAiIsActive = !!localAiStatus?.configured | ||
| && !!aiProvider?.provider | ||
| && (aiProvider.provider === HERMES_LOCAL_PROVIDER_ID || aiProvider.provider === localAiStatus.provider); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the active model before marking Local AI as selected.
clawlocal identifies the Hermes local-provider slot. It does not identify a specific local model. This condition reports the configured model as active whenever the provider matches, even if aiProvider.model still selects another local model. The panel can then show standby or running state and hide the switch action incorrectly.
Compare the normalized active model with localAiStatus.model in addition to the provider.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/SettingsApp.tsx` around lines 1046 - 1048, Update the
localAiIsActive condition in SettingsApp so it compares the normalized active
model against localAiStatus.model in addition to the existing configured and
provider checks; keep the Hermes provider-slot matching behavior while ensuring
a different local model does not mark Local AI as selected.
| // Installed is not selected. Without this the panel rendered the | ||
| // green "already configured" pill and hid its own switch button, | ||
| // so a device that had Gemma installed but unselected offered no | ||
| // way to actually start using it. | ||
| localAiIsActive={localAiIsActive} | ||
| title="Set Up Local AI" | ||
| description={localAiStatus?.configured | ||
| ? "Gemma 4 is configured as your private on-device fallback." | ||
| ? "Gemma 4 is installed as your private on-device model." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Refresh aiProvider after local-model activation.
localAiIsActive derives from aiProvider, but the onConfigured callback refreshes only localAiStatus. After a successful switch, this component retains the old active-provider result until remount. The panel can continue to show “Switch to Gemma 4” after Hermes or OpenClaw already selected it.
Fetch /setup-api/ai-models/status and update aiProvider in this callback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/SettingsApp.tsx` around lines 2414 - 2421, Update the
local-model activation onConfigured callback in SettingsApp so it refreshes both
localAiStatus and aiProvider after success. Fetch /setup-api/ai-models/status in
that callback, derive the current provider from the response, and update
aiProvider so the panel immediately reflects Gemma 4 activation without
requiring a remount.
| const existingPid = await llamaCpp.readLlamaCppPid(spec.pidPath) | ||
| if (existingPid && llamaCpp.isLlamaCppPidRunning(existingPid)) { | ||
| console.log(`[instrumentation] llama.cpp already starting for ${alias} (pid=${existingPid})`) | ||
| return | ||
| return 'started' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Bind the existing process to the requested alias.
Line 188 checks only PID liveness. It does not prove that the process is starting alias.
If a stale process starts a prior alias, this branch returns "started" for the new alias. The runtime then treats the wrong server as ready or waits for an alias that the process will not load.
Persist the launch alias with the PID, or inspect the process arguments. Return "started" only when the active launch matches alias.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/instrumentation-node.ts` around lines 188 - 191, Update the
existing-process branch in the instrumentation startup flow to verify that the
live PID belongs to the requested alias before returning "started". Persist and
read the launch alias alongside the PID, or inspect the active process
arguments, and only return "started" when it matches alias; otherwise continue
with the appropriate new-launch handling.
| const provisioning = await getLlamaCppProvisioningStatus(alias).catch(() => null); | ||
| budgetMs = provisioning?.modelAvailable ? getLlamaCppWakeTimeoutMs() : spec.startupTimeoutMs; | ||
| deadline = Date.now() + budgetMs; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Use complete provisioning state for the readiness budget.
modelAvailable does not prove that llama.cpp is ready for a wake. If the model file exists but binPath is absent, startup still needs binary provisioning. The current branch limits that work to the 180-second wake budget.
src/lib/local-ai-runtime.ts#L127-L129: select the wake timeout only whenprovisioning?.installedis true.src/tests/unit/local-ai-runtime-failfast.test.ts#L95-L105: setinstalled: truefor the wake fixture and add coverage for a missing binary.
🧰 Tools
🪛 ast-grep (0.45.1)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile as execFileCb } from "child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
📍 Affects 2 files
src/lib/local-ai-runtime.ts#L127-L129(this comment)src/tests/unit/local-ai-runtime-failfast.test.ts#L95-L105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/local-ai-runtime.ts` around lines 127 - 129, Use
provisioning.installed, rather than provisioning.modelAvailable, when selecting
the wake timeout in the local-ai-runtime readiness budget. In
src/lib/local-ai-runtime.ts lines 127-129, preserve the startup timeout unless
installed is true; in src/tests/unit/local-ai-runtime-failfast.test.ts lines
95-105, set installed: true on the wake fixture and add coverage verifying a
missing binary uses the startup budget.
1 participant
What was wrong
On a device whose agent configuration does not live in OpenClaw, the on-device model could not be started at all, and Settings claimed it was in use when it was not.
1. The start gate could never open.
bootLlamaCppServer()decided whether to launch by readingagents.defaults.model.primaryout of the OpenClaw config. On editions that ship without OpenClaw that key is never written — the configure route returns early long before it — so the gate was permanently closed. Nothing spawned, and the journal recorded the same line every time:2. The caller then waited 20 minutes for it.
ensureLocalAiReady()could not tell a start that never happened from one that did, so it went straight into a readiness poll bounded by the download budget. A chat with the on-device model selected hung for ten minutes and then failed. Because the doomed attempt stayed memoized instartPromise, every later request joined the same dead wait — the failure outlived its cause.3. Settings said it was in use when it was only installed. The Local AI card was built entirely from the
local_ai_configured/local_ai_modelconfig-store keys, which only ever prove "installed and available". Enabling a local model deliberately does not take over from the provider the customer picked, so the card readGemma 4 Local · sleeping until neededwhile every message went to a cloud provider.What changed
spawned/already-running/already-starting/skipped-*), and a no-op fails immediately with an explainable message instead of polling.LLAMACPP_WAKE_TIMEOUT_MS./setup-api/ai-models/statusendpoint that already resolves this for both harnesses — so there is no second implementation and the runtime-health routes are untouched. It saysavailable, not currently selectedand names what is answering instead. The four card states are resolved once, as data, so the status line and the card copy cannot drift.llamacpp-serverand is now shared by the launcher and the wake path. They previously disagreed: the wake path read only the OpenClaw config, so on editions where that is silent it fell through to the default alias and a device configured with a non-default local model woke the wrong one.Verification
Run on a Jetson Orin Nano, on the edition that reproduced the fault.
Unit suite on hardware: 188 files / 2226 tests, 0 failures (baseline 184 / 2200).
tsc --noEmitadds no new errors — the same 2 pre-existing ones before and after — andlintis byte-identical to the base branch (93 problems, all pre-existing).Cold wake through the real chat path, with no
llama-serverprocess running:auto-start skipped (…)auto-starting gemma4-e2b-it-q4_0 (pid=…)Fail-fast, verified by making the runtime unable to come up: the proxy returned
502 {"error":"Timed out after 180s waiting for llama.cpp (…) to become ready"}instead of holding the request for 20 minutes.Activation truth, from the endpoint that drives the card:
Memory stayed within budget throughout — peak 4392 MB of 7607 MB with the model resident.
Summary by CodeRabbit
New Features
Bug Fixes
Tests