Feature/ollama local provider - #12
Merged
Merged
Conversation
- Add Ollama to setup wizard and dashboard AI provider selection - Add backend routes for Ollama status check and model pulling with streaming progress - Add Ollama installation step to install.sh and system updater pipeline - Configure OpenClaw gateway for Ollama with dummy auth profile (no API key needed) - Model options: Llama 3.2 3B, Qwen2.5 3B Instruct (Q4_K_M) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add /setup-api/ollama/search endpoint that queries Ollama library - Filter results to models with ≤8B parameters (fits Jetson Orin Nano) - Add debounced search input in both setup wizard and dashboard - Search results show size buttons to select specific model variants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use api_key mode with dummy key for Ollama auth profile (OpenClaw rejects mode "none") - Track saving state per model name so only the clicked Use button shows spinner - Remove dangerouslyDisableDeviceAuth (breaks existing device identity) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Ollama local-model support (routes, hook, UI), introduces an OpenClaw device-identity bypass patch in the installer, removes the voice pipeline install step, and makes updater branch-aware via a pinned Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Frontend UI
participant Backend as Backend API
participant Ollama as Ollama Server
participant Gateway as OpenClaw Gateway
User->>Frontend: Select Ollama / search or pull model
Frontend->>Backend: GET /setup-api/ollama/status
Backend->>Ollama: GET /api/tags (5s timeout)
Ollama-->>Backend: models / error
Backend-->>Frontend: { running, models }
User->>Frontend: Search models
Frontend->>Backend: GET /setup-api/ollama/search?q=...
Backend->>Ollama: fetch HTML library (10s timeout)
Ollama-->>Backend: HTML
Backend-->>Frontend: filtered model list
User->>Frontend: Pull model
Frontend->>Backend: POST /setup-api/ollama/pull { model }
Backend->>Ollama: POST /api/pull (streaming)
Ollama-->>Backend: NDJSON stream (progress/errors)
Backend-->>Frontend: NDJSON streamed progress
Frontend->>Backend: POST /setup-api/ai-models/configure (save Ollama config)
Backend->>Gateway: (optional) disable device auth / apply patch
Gateway-->>Backend: ack / status
Backend-->>Frontend: config saved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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)
Comment |
✅ Test Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
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)
119-125: 🧹 Nitpick | 🔵 TrivialError message is slightly misleading for Ollama provider.
When the provider is not Ollama and apiKey is missing, the error says "Provider and API key are required" which is correct. However, the logic correctly allows Ollama without an API key. Consider a minor improvement to the error message for clarity.
💡 Consider improving error message
const isOllama = provider === "ollama"; if (!provider || (!apiKey && !isOllama)) { return NextResponse.json( - { error: "Provider and API key are required" }, + { error: isOllama ? "Provider is required" : "Provider and API key are required" }, { status: 400 } ); }Actually, this branch can only be reached when
!provideris true (since!isOllamais already handled), so the current message is acceptable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/setup-api/ai-models/configure/route.ts` around lines 119 - 125, The error message returned from the validation block using provider, apiKey and isOllama is misleading for the Ollama case; update the NextResponse.json error string to clearly state that a provider is required and that an API key is only required for non-ollama providers (e.g. "Provider is required; API key required for non-ollama providers") so callers understand Ollama doesn't need an apiKey—adjust the message in the block that checks if (!provider || (!apiKey && !isOllama)) where NextResponse.json is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.sh`:
- Around line 473-474: The two commands "systemctl enable ollama" and "systemctl
start ollama" are currently silencing failures; change them to capture and check
their exit status, log failures, and verify service health before continuing:
run the enable and start commands without the "2>/dev/null || true" suppression,
test their exit codes (or use "systemctl is-active --quiet ollama" / "systemctl
status ollama" to confirm the service is running), write a clear diagnostic
message to stdout/stderr when enable/start fails, and decide whether to abort or
continue based on that check so configuration/startup failures aren't silently
ignored.
- Around line 461-475: In step_ollama_install replace the unbounded pipe "curl
-fsSL https://ollama.com/install.sh | sh" with a timed download and explicit
execution: use curl with --connect-timeout and --max-time (e.g.,
--connect-timeout 10 --max-time 60) to write the script to a temp file, check
curl's exit status, then run sh on that temp file and check its exit status; if
either fails, emit a clear error (instead of the current generic message) and
exit non-zero. Also avoid piping directly from curl to sh to ensure the timeout
is effective and make sure the existing checks around command -v ollama and the
systemctl enable/start calls remain unchanged.
In `@src/app/setup-api/ollama/pull/route.ts`:
- Around line 7-15: The POST handler currently accepts model from request.json()
and passes it to Ollama without validation; add a format check for the model
variable (e.g., require the "name:tag" pattern) right after body parsing and
before using model, using a simple regex (for example
/^[a-z0-9._-]+:[a-zA-Z0-9._-]+$/i) to validate the string, and if it fails
return NextResponse.json({ error: "Invalid model format, expected name:tag" }, {
status: 400 }); update the logic around the model constant in the POST function
so invalid formats are rejected with a clear message rather than forwarded to
Ollama.
- Around line 72-78: Remove the redundant "Transfer-Encoding": "chunked" header
from the Response returned in the handler that constructs new Response(stream,
...); keep "Content-Type": "application/x-ndjson" and "Cache-Control":
"no-cache" and let the runtime handle chunking for the ReadableStream named
stream in route.ts.
In `@src/app/setup-api/ollama/search/route.ts`:
- Around line 25-65: The parseSearchResults function relies on fragile regex
HTML scraping (see parseSearchResults, modelBlockRe, nameMatch, descMatch,
tagMatches, sizeMatches); add a clear comment above parseSearchResults noting
the dependency on Ollama's HTML structure and the risk of breakage, implement
graceful degradation by returning partial results when some fields fail to parse
(e.g., keep name even if desc/pulls/tags missing) and log parsing failures via
the existing logger, and consider adding a short in-memory cache around the
function call (TTL e.g., 30–60s) to reduce external requests and improve
resilience.
In `@src/components/AIModelsStep.tsx`:
- Around line 252-358: Extract the duplicated Ollama logic into a reusable hook
named useOllamaModels that encapsulates state (ollamaRunning, ollamaModels,
ollamaSearch, ollamaSearchResults, ollamaSearching, ollamaPulling,
ollamaPullProgress, ollamaSaving, status, searchTimerRef) and the functions
checkOllamaStatus, searchOllamaModels, handleOllamaSearchChange,
pullOllamaModel, saveOllamaConfig, selectExistingOllamaModel and utility
formatOllamaBytes; move the fetch/streaming logic and error handling into the
hook, return the state values and handlers from useOllamaModels, then replace
the in-file implementations in AIModelsStep.tsx and DoneStep.tsx with calls to
the hook and update imports/usages to use the returned functions and state.
In `@src/components/DoneStep.tsx`:
- Around line 387-397: Add a cleanup to the existing useEffect that manages
ollama search debouncing: if ollamaSearchTimerRef.current is set, call
clearTimeout(ollamaSearchTimerRef.current) in the effect's return cleanup and
then set ollamaSearchTimerRef.current = null to avoid stale timers attempting
state updates after unmount; update the useEffect that references
ollamaSearchTimerRef to include this cleanup so the timer is cleared when the
component unmounts or the effect re-runs.
---
Outside diff comments:
In `@src/app/setup-api/ai-models/configure/route.ts`:
- Around line 119-125: The error message returned from the validation block
using provider, apiKey and isOllama is misleading for the Ollama case; update
the NextResponse.json error string to clearly state that a provider is required
and that an API key is only required for non-ollama providers (e.g. "Provider is
required; API key required for non-ollama providers") so callers understand
Ollama doesn't need an apiKey—adjust the message in the block that checks if
(!provider || (!apiKey && !isOllama)) where NextResponse.json is returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0d7de858-132f-4e1f-8df6-096adaafcf63
📒 Files selected for processing (8)
install.shsrc/app/setup-api/ai-models/configure/route.tssrc/app/setup-api/ollama/pull/route.tssrc/app/setup-api/ollama/search/route.tssrc/app/setup-api/ollama/status/route.tssrc/components/AIModelsStep.tsxsrc/components/DoneStep.tsxsrc/lib/updater.ts
- Extract shared useOllamaModels hook from AIModelsStep and DoneStep - Add device identity bypass patch to installer for OpenClaw bug - Add model name validation and search result caching - Remove Ollama auto-install from installer (managed via dashboard) - Remove voice pipeline (Whisper/Kokoro) from installer - Remove openclaw_models and ollama steps from updater Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the checked-out branch tracks a remote (e.g. feature/ollama-local-provider), the updater now pulls that branch instead of switching to main. Untracked or local-only branches still fall back to main. Includes branch name validation to prevent shell injection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@install.sh`:
- Line 510: The TOTAL_STEPS constant is out of sync with the install flow;
update the TOTAL_STEPS variable so progress reflects the actual number of
log-emitting steps (or derive it dynamically). Either change TOTAL_STEPS to 18
to match the current run, or compute TOTAL_STEPS from the canonical list/array
of steps used by the script (so it always equals the number of times the install
emits a log) and replace the hardcoded TOTAL_STEPS; ensure this ties to the same
step source that produces the log calls (the code that invokes log).
- Around line 253-256: The installer currently only warns when the device-auth
patch verification (grep for DEVICE_MARKER in GATEWAY_DIST) fails; change the
else branch so the script fails the update by logging a clear error and exiting
with a non-zero status (e.g., exit 1) instead of printing a warning. Locate the
conditional that checks grep -qrl "$DEVICE_MARKER" "$GATEWAY_DIST" and replace
the warning message in the else block with a descriptive error via echo/process
logger and an exit 1 to abort the install when the patch cannot be verified.
In `@src/app/setup-api/ai-models/configure/route.ts`:
- Around line 141-145: The Ollama branch reuses `apiKey` as a model name which
is confusing; add a concise inline comment above the block that checks
`isOllama` explaining that for Ollama the front-end/consumer supplies the model
name via the `apiKey` field (e.g., `llama3.2:3b`) and that `modelName` is
derived from `apiKey` before assigning `config.defaultModel =
\`ollama/${modelName}\`` so future maintainers understand the semantic reuse of
`apiKey` (references: isOllama, apiKey, modelName, config.defaultModel).
- Around line 234-243: When the Ollama branch sets
gateway.controlUi.dangerouslyDisableDeviceAuth to true (inside the isOllama
check using runCommand and OPENCLAW_BIN in route.ts), add a clear info-level log
entry immediately before or after the runCommand call (e.g., via the existing
logger/processLogger) stating that device authentication is being disabled for
Ollama, include context like the provider name and that this is intentional for
local use, and ensure the log message is human-readable and auditable so
operators can detect this security-sensitive change; also add a brief TODO
comment pointing to user docs for this behavior.
In `@src/app/setup-api/ollama/pull/route.ts`:
- Around line 50-69: The loop currently enqueues the raw chunk
(controller.enqueue(value)) before checking for an error, causing duplicate
error output; change the logic in the stream read loop that uses reader,
controller, decoder, value and text so you decode the chunk first, parse lines
to detect a parsed.error, and only enqueue either the original value (if no
error found) or a single error JSON and close the controller (if an error is
found); alternatively, if the original chunk already contains an error JSON,
skip enqueuing the extra error wrapper to avoid sending the same error twice.
In `@src/app/setup-api/ollama/search/route.ts`:
- Around line 141-146: The cache insertion currently does
searchCache.set(cacheKey, { results, ts: Date.now() }) then evicts, allowing a
transient size of 51; change the logic in the route where searchCache, cacheKey
and results are used to evict before adding: check if searchCache.size >= 50 (or
loop while > = limit) and delete the oldest entry determined by
[...searchCache.entries()].sort((a,b)=>a[1].ts-b[1].ts)[0] (using the same ts
field) prior to calling searchCache.set(cacheKey, { results, ts: Date.now() });
this guarantees the cache never exceeds the configured limit.
In `@src/components/DoneStep.tsx`:
- Around line 1419-1546: The Ollama UI block is duplicated between DoneStep and
AIModelsStep; extract it into a shared component (e.g., OllamaModelPanel) and
replace the duplicated JSX with that component. The new component should accept
props/state handlers used here: ollamaRunning, ollamaModels, saveOllamaConfig,
ollamaSaving, selectedOllamaModel, setSelectedOllamaModel,
handleOllamaSearchChange, ollamaSearch, ollamaSearching, ollamaSearchResults,
clearSearch, pullOllamaModel, ollamaPulling, ollamaPullProgress,
formatOllamaBytes and preserve the same button texts/disabled states and
progress UI; then import and use OllamaModelPanel in DoneStep and AIModelsStep
to remove duplication.
- Line 1437: The label element with text "Download a model" using LABEL_CLASS is
not associated with a form control; either convert it to a semantic heading
(e.g., replace the <label> with an appropriate heading element using
LABEL_CLASS) or associate it with the radio group by giving the label an id and
setting the radio group's container to role="group" and
aria-labelledby="<that-id>" (update the radio group's JSX where the model
selection radios are rendered). Ensure the unique identifier used for
aria-labelledby matches the label's id and keep LABEL_CLASS for styling.
In `@src/hooks/useOllamaModels.ts`:
- Around line 48-58: In checkOllamaStatus (the useCallback in
useOllamaModels.ts), avoid calling res.json() unconditionally: first check
res.ok and only parse JSON when true; if !res.ok, setOllamaRunning(false) and
setOllamaModels([]) (optionally read res.text() for logging/debugging) and bail
out so JSON parsing errors from HTML/error pages are prevented; keep the
existing catch to handle network/parse errors.
- Around line 60-77: The searchOllamaModels function lacks a check of the fetch
response status before calling res.json(); update searchOllamaModels to verify
res.ok after the fetch and before parsing JSON, and handle non-OK responses by
setting setOllamaSearchResults([]) (and optionally logging or setting an error
state) instead of calling res.json() on an error response; ensure
setOllamaSearching(false) still runs in the finally block and keep the existing
behavior of clearing results on error by using the same error path used in the
catch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 98697345-8739-4205-a56a-f4f11fb17653
📒 Files selected for processing (8)
install.shsrc/app/setup-api/ai-models/configure/route.tssrc/app/setup-api/ollama/pull/route.tssrc/app/setup-api/ollama/search/route.tssrc/components/AIModelsStep.tsxsrc/components/DoneStep.tsxsrc/hooks/useOllamaModels.tssrc/lib/updater.ts
💤 Files with no reviewable changes (1)
- src/lib/updater.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/updater.ts`:
- Around line 80-115: The current logic in updater.ts uses branch.<name>.remote
but always resets to origin/${currentBranch} and swallows git errors; change the
update flow in the block using execShell, gitCmd, currentBranch and SAFE_BRANCH
so you first attempt to resolve the actual upstream ref with git rev-parse
--abbrev-ref --symbolic-full-name @{u} (via execShell) and, if successful, use
that resolved upstream (e.g., remote/merge ref) in the reset --hard command
instead of hardcoding origin/${currentBranch}; handle the case where rev-parse
reports “no upstream” separately (fall back to switching to main) and
propagate/log unexpected execShell errors instead of silently catching them
(remove the empty catch and distinguish expected “no upstream” output from other
failures).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 69146061-bb81-4c5a-a78e-051579a2fcb0
📒 Files selected for processing (1)
src/lib/updater.ts
The updater now resolves which branch to pull in priority order: 1. .update-branch file (pinned branch, survives factory reset + git reset) 2. Current branch if it tracks a remote 3. "main" as fallback Adds /setup-api/system/update-branch API endpoint and branch selector in the System Update dialog so users can pin a branch from the dashboard. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/components/DoneStep.tsx (1)
1258-1267:⚠️ Potential issue | 🟡 MinorAssociate these section labels with their actual controls.
Update branchis rendered as a free-standing<label>withouthtmlFor, andDownload a modelis section text rather than a label for the radio group. Bind the first label to the text input and use afieldset/legendoraria-labelledbyfor the Ollama model radios.As per coding guidelines,
src/components/**: React 19 components with Tailwind CSS v4. Review for accessibility, proper state management, and XSS prevention.Also applies to: 1502-1514
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DoneStep.tsx` around lines 1258 - 1267, The "Update branch" label is not associated with the text input—add a unique id (e.g., branch-input-id) to the input bound to the existing branchInput/setBranchInput state and set the label's htmlFor to that id; for the Ollama model radio group, wrap the radios in a <fieldset> with a <legend> or give the group a container with aria-labelledby pointing to a visible label id, and ensure each radio input has its own id and corresponding <label htmlFor=> so the radios are properly announced and clickable; apply the same pattern to the similar block referenced by branchInput usage around the 1502-1514 area.src/lib/updater.ts (1)
84-109:⚠️ Potential issue | 🔴 CriticalThis still resets against
origin/${targetBranch}instead of the configured upstream.
resolveUpdateBranch()only checks whetherbranch.<name>.remoteexists, special-cases"main", andupdateClawBoxAndReboot()still hard-resets toorigin/${targetBranch}. Branches tracking a non-originremote or a differently named upstream ref will be reset to the wrong commit, and the empty catches keep unexpected Git failures on the fallback path. Resolve@{u}explicitly and only fall back when Git reports that no upstream is configured.As per coding guidelines,
src/lib/**: TypeScript server-side libraries. Review for proper error handling and type safety.Also applies to: 116-125
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/updater.ts` around lines 84 - 109, resolveUpdateBranch currently only checks branch.<name>.remote and silent-catches failures, and updateClawBoxAndReboot still does a hard reset to origin/${targetBranch}; change resolveUpdateBranch to determine the actual configured upstream for the current branch by running git to resolve the upstream ref (e.g. use `${gitCmd} rev-parse --abbrev-ref --symbolic-full-name ${current}@{u}` or an equivalent git command) and validate that result against SAFE_BRANCH before returning it (fall back to "main" only when git explicitly reports no upstream), remove the empty catches so unexpected git errors are propagated or logged, and update updateClawBoxAndReboot to hard-reset to the resolved upstream ref instead of `origin/${targetBranch}`; reference resolveUpdateBranch, updateClawBoxAndReboot, UPDATE_BRANCH_FILE, and SAFE_BRANCH when making the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/setup-api/system/update-branch/route.ts`:
- Around line 11-17: GET currently treats any fs error as "no pinned branch";
change the error handling around readFile(UPDATE_BRANCH_FILE, "utf-8") to only
swallow ENOENT (return { branch: null } with 200) and return a 500 JSON error
for any other exceptions (inspect err.code === "ENOENT"). Apply the same pattern
to the delete/unlink path (where unlink(UPDATE_BRANCH_FILE) is used): if unlink
throws ENOENT treat as success/cleared, but for other errors return a 500
response. Update the code in the GET function and the corresponding unlink
handler to use explicit error.code checks and appropriate NextResponse status
codes.
- Around line 9-10: Replace the brittle SAFE_BRANCH regex validation with a real
Git ref validation: call out to git (e.g., spawnSync/execSync "git remote" to
list remotes and "git check-ref-format --branch <ref>") from the update-branch
route and the updater module (where SAFE_BRANCH is also used) to validate input;
if the input begins with a remote prefix (detect by comparing the first path
segment against git remote names) strip that leading remote segment before
running git check-ref-format and before persisting to .update-branch, and reject
any ref for which git check-ref-format exits non‑zero; finally remove or replace
the SAFE_BRANCH constant usage in both the update-branch route and the updater
module so all branch validation/normalization goes through this git-based flow.
In `@src/components/DoneStep.tsx`:
- Around line 619-634: The saveUpdateBranch function currently swallows POST
failures and allows triggerUpdate to run against stale branch state; change the
flow so the Update action either awaits saveUpdateBranch and only calls
triggerUpdate when saveUpdateBranch resolves OK (setUpdateBranch matches the
current input) or disable the Update button while setBranchSaving is true or
when updateBranch !== current input value; also surface POST errors from
saveUpdateBranch (don’t use an empty catch) by returning success/failure or
throwing the error so the UI can show a validation/error message and prevent
triggerUpdate from running against a stale branch (refer to saveUpdateBranch,
triggerUpdate, setBranchSaving, setUpdateBranch and the input state).
---
Duplicate comments:
In `@src/components/DoneStep.tsx`:
- Around line 1258-1267: The "Update branch" label is not associated with the
text input—add a unique id (e.g., branch-input-id) to the input bound to the
existing branchInput/setBranchInput state and set the label's htmlFor to that
id; for the Ollama model radio group, wrap the radios in a <fieldset> with a
<legend> or give the group a container with aria-labelledby pointing to a
visible label id, and ensure each radio input has its own id and corresponding
<label htmlFor=> so the radios are properly announced and clickable; apply the
same pattern to the similar block referenced by branchInput usage around the
1502-1514 area.
In `@src/lib/updater.ts`:
- Around line 84-109: resolveUpdateBranch currently only checks
branch.<name>.remote and silent-catches failures, and updateClawBoxAndReboot
still does a hard reset to origin/${targetBranch}; change resolveUpdateBranch to
determine the actual configured upstream for the current branch by running git
to resolve the upstream ref (e.g. use `${gitCmd} rev-parse --abbrev-ref
--symbolic-full-name ${current}@{u}` or an equivalent git command) and validate
that result against SAFE_BRANCH before returning it (fall back to "main" only
when git explicitly reports no upstream), remove the empty catches so unexpected
git errors are propagated or logged, and update updateClawBoxAndReboot to
hard-reset to the resolved upstream ref instead of `origin/${targetBranch}`;
reference resolveUpdateBranch, updateClawBoxAndReboot, UPDATE_BRANCH_FILE, and
SAFE_BRANCH when making the changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c1177f6d-a5ca-40f2-ab30-fbe28870ba8c
📒 Files selected for processing (4)
.gitignoresrc/app/setup-api/system/update-branch/route.tssrc/components/DoneStep.tsxsrc/lib/updater.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 3: The package.json "version" field was incorrectly bumped as a patch;
change the "version" value from "2.2.2" to "2.3.0" to reflect the added features
(Ollama integration and update-branch) and update any internal references or
CI/release scripts that read the package.json "version" field so they remain
consistent with the new minor bump.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 652b67bf-5a6b-419f-b35a-93cb0a276baa
📒 Files selected for processing (1)
package.json
- Fix TOTAL_STEPS count (20→18) to match actual install flow - Exit on device-auth patch verification failure instead of warning - Add comment explaining apiKey→model name semantic reuse for Ollama - Add log + TODO for dangerouslyDisableDeviceAuth security-sensitive change - Fix duplicate error output in pull stream (decode before enqueue) - Fix cache eviction order: evict before insert to stay within limit - Extract shared OllamaModelPanel component (removes ~250 lines duplication) - Replace <label> with <h4> for "Download a model" heading (accessibility) - Add res.ok checks before JSON parsing in useOllamaModels hook Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Only swallow ENOENT in update-branch GET/unlink, return 500 for other errors
- Surface branch save errors in UI, disable Update button while saving
or when input doesn't match saved branch state
- Add htmlFor association on branch input label
- Use git rev-parse @{u} to resolve actual upstream ref instead of
assuming origin/<branch>, making the updater work with non-origin remotes
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The branch selector in the System Update dialog is now only visible when a branch pin exists or the version indicates non-tag commits (dev build). On main at a release tag, users just see the clean Update button. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The patch checked globally for the marker and returned early if found in ANY file. After an OpenClaw update, the old (already patched) file still exists alongside a new unpatched file. Now checks each file individually and only patches files missing the marker. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Button now shows spinner + "Configuring..." after download completes while the Ollama config is being saved to OpenClaw. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The "gateway token missing" error was a separate issue from device identity: gateway.auth.mode=token requires a token the browser can't provide over HTTP. Set auth.mode=none since the gateway is only reachable via the local Next.js proxy. Applied in both the installer (step_openclaw_config) and the AI configure route (for all providers, not just Ollama). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Ollama pre-allocates KV cache for the full context window. The default 128K context on a 3B model requires ~12.5GB, exceeding the Jetson's 8GB. Write models.json with contextWindow: 8192 when configuring Ollama. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add DELETE endpoint at /setup-api/ollama/delete - Add trash icon next to "Use" button for each installed model - Patch models.json contextWindow after gateway restart instead of creating custom model variants (OpenClaw overrides Modelfile num_ctx) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Poll models.json with retry instead of blind 2s sleep for context window patch - Only patch ollama provider models (not all providers) with Array.isArray guard - Use is-enabled instead of is-active for snapd check in installer - Rename Qwen preset label to "Qwen2.5 3B" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…timizations - Define Ollama model in openclaw.json with models.mode=replace so the gateway uses our 16K context cap instead of auto-detecting 128K from Ollama - Reset models.mode to merge when switching to cloud providers - Add optimize-ollama.sh script (q8_0 KV cache, flash attention, single model) that halves KV cache memory on 8GB Jetson - Add sudoers rule so web UI can run optimize-ollama.sh via sudo - Remove fragile post-restart models.json patching (race condition with gateway) - Installer runs optimize-ollama.sh and installs sudoers during setup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Closed
This was referenced Apr 20, 2026
Merged
This was referenced Apr 28, 2026
Merged
yalexx
added a commit
that referenced
this pull request
Sep 6, 2026
…740) * security: close the CodeRabbit deep-scan findings that still hold on beta The 2026-09-05 scan of main reported 23 findings; each was re-verified against beta before anything changed. Five were already fixed on beta (#1 #2 #5 #8 #18), three are the appliance's documented design (#3 #13 #15), two need a design decision rather than a patch (#12 the self-updating root steps, #16 system_power via the bearer) and are deferred with their designs written up. This closes the rest: - #21/#8: root units (clawbox-ap, ap-watchdog, the NM failover hook, first-boot VNC, recover) run the root-owned /usr/local/libexec/clawbox copies and load /etc/clawbox/network.env, never the clawbox-owned tree; clawbox-heartbeat runs as User=clawbox; a class-wide test pins the rule. - #11: the Files API refuses to rename or delete a protected container (data/, the checkout, ~/.config, the browse root) — protected_container. - #19: the MCP path guard judges the canonical path (nearest existing ancestor) as well as the typed one, and the file tools open the vetted target with O_NOFOLLOW. - #17: the webapp document carries a sandbox CSP wherever it is opened (shipped through next.config.ts, since a route header is dropped in production), and installed_* preference writes are owner-only. - #20/#22: clawkeep restore derives every destination on the box and refuses the manifest's before anything moves; link members must resolve inside the staging root; restore/unpair/snapshot/encryption/reset-state are owner-only and same-origin. - #7: CF-Connecting-IP and its siblings are stripped unless the socket peer is loopback (cloudflared's), so a LAN client cannot pick its lockout bucket. - #4: regex code search is gone (400 regex_unsupported). - #6: uploads are bounded by a free-space reserve with busboy limits and partials unlinked; the attachments route gets the same teardown deferral. - #14: the Kokoro/Whisper sockets are 0600 with SO_PEERCRED, and Kokoro's output path is confined to a .wav regular file under /tmp. - #9 (part): the MCP server scrubs CLAWBOX_MCP_TOKEN from its environment at startup; allow_dangerous is documented as a typo override, not consent. - #10: issue-triage/pr-review validate the model's JSON on both transports, derive labels from fixed tables and sanitise comment text. - #23: e2e-install writes repository secrets only off pull_request events. - #1/#5 residuals: setup/complete checks the session in-handler; the middleware matcher no longer skips /fonts/ and /images/. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuyrrYnKgrUkBXECWqW1gb * fix: vouch for the path at the two sinks CodeQL flagged The multipart cleanup unlinked paths whose containment check governed the write inside the promise, not the catch block; and the dangling-link resolver lstat/readlink'd a name straight off the caller's path. Both now resolve and prefix-check right before the call, the shape safePath already uses (js/path-injection alerts 519-521). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuyrrYnKgrUkBXECWqW1gb * fix: address CodeRabbit's review of the security sweep - e2e-install: the one job names its Environment by event (e2e-credentials off pull_request, an empty e2e-pull-request on one), documented for the owner; the schema strip for the SDK transport is schema-aware and covers Anthropic's whole unsupported set, and the local validator refuses any constraint it cannot check so no cap is silently unenforced. - clawkeep: a Hermes sessions asset that omits sqlite still retires the sidecars (the box's own flag wins); OPENCLAW_STATE_DIR placeholders count as unset; the no-state fallback matches both CLI message forms, with one shared recorded-CLI fixture. - install.sh: a libexec copy that did not land is never a success — collected, recorded as root_libexec, and the units that name the copies are not written over it. - root-unit tests parse User= (User=root is root) and refuse /home/clawbox anywhere in a directive value; the code search route refuses a non-string pattern; notebook_edit has its symlink regression case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuyrrYnKgrUkBXECWqW1gb * test: give the libexec test in root-steps both ceilings It runs install_root_libexec under a real bash, and the timeout-hygiene rule (test-timeout-hygiene.test.ts) asks every spawning suite for a declared testTimeout and hookTimeout — the one CI failure on the previous commit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SuyrrYnKgrUkBXECWqW1gb --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Changes
UX