v2.2.1: Bug fixes, hotspot toggle, MAXN power mode, code quality impr… - #11
Merged
Merged
Conversation
…ovements - Fix nvpmodel: dynamically detect MAXN mode instead of hardcoding mode 0 (15W) - Add hotspot enable/disable toggle in dashboard Security section - Add WiFi warning about hotspot disconnection in setup wizard and dashboard - Fix ClawBox version comparison: extract base tag from git describe output - Fix OpenClaw version detection: fallback to package.json, dedup current=target - Fix disk total: calculate from df output instead of hardcoded "512 GB" - Fix OAuth start: return 400 (not 500) for missing Google config - Simplify OAuth exchange timeout: use AbortSignal.timeout() - Simplify gateway proxy: use AbortSignal.timeout(), fix HTML escape ordering - Optimize hotspot route: use getAll()/setMany() to reduce config file I/O - Optimize OpenClaw version fallback: use fs.readFile instead of spawning node - Fix start-ap.sh: use $IFACE variable, add hotspot disabled check - Fix install.sh: Node.js version regex, single-pass nvpmodel.conf parsing - Add missing force-dynamic export to update/run route 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:
📝 WalkthroughWalkthroughDynamic Jetson power-model selection replaces hardcoded mode; hotspot enable/disable added across UI, API, and scripts with explicit stop/start flows; AbortSignal.timeout replaces manual AbortController timeouts; disk reporting now includes diskTotal; updater gains package.json fallback and base-tag normalization. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant UI as DoneStep/WifiStep
participant API as Hotspot API
participant Config as Config Store
participant Scripts as System Scripts
User->>UI: Toggle hotspot (enable/disable)
UI->>API: POST /api/system/hotspot {ssid,password,enabled}
API->>Config: setMany({hotspot_ssid,hotspot_password,hotspot_enabled})
Config-->>API: ack
alt enabled == true
API->>Scripts: systemctl start clawbox-root-update (restart_ap)
Scripts-->>API: service started
else enabled == false
API->>Scripts: execute stop-ap.sh
Scripts-->>API: AP stopped
end
API-->>UI: 200 OK
UI->>API: GET /api/system/hotspot
API->>Config: getAll()
Config-->>API: hotspot config + enabled
API-->>UI: {ssid, hasPassword, enabled}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
🚥 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/clawbox-performance.service`:
- Line 8: The ExecStart command uses a strict regex lookahead that only matches
NAME=MAXN, causing MAXN_SUPER entries to be ignored; update the grep pattern
used to compute MAXN_ID (the command that sets MAXN_ID from /etc/nvpmodel.conf)
so it matches both MAXN and MAXN_SUPER (i.e., mirror install.sh behavior of
grepping for 'NAME=MAXN' as a substring), then leave the rest of the pipeline
(tail -1; nvpmodel -m ${MAXN_ID:-0} && jetson_clocks) unchanged so the service
picks the highest MAXN variant at boot consistent with install.sh.
In `@src/lib/system-info.ts`:
- Line 57: Extract the repeated fallback object into a single constant (e.g.,
UNKNOWN_DISK or UNKNOWN_DISK_INFO) and replace the duplicate literal returns
with that constant to avoid drift; update both places that currently return {
diskTotal: "unknown", diskUsed: "unknown", diskFree: "unknown", diskUsedPercent:
0 } (the return in the function that currently references
diskTotal/diskUsed/diskFree/diskUsedPercent and the second identical return) and
reference the new constant instead, placing the constant near the top of
src/lib/system-info.ts so both functions can use it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d722ec7d-e66b-4337-ae64-1cf0272f613b
📒 Files selected for processing (12)
config/clawbox-performance.serviceinstall.shscripts/start-ap.shsrc/app/setup-api/ai-models/oauth/exchange/route.tssrc/app/setup-api/ai-models/oauth/start/route.tssrc/app/setup-api/system/hotspot/route.tssrc/app/setup-api/update/run/route.tssrc/components/DoneStep.tsxsrc/components/WifiStep.tsxsrc/lib/gateway-proxy.tssrc/lib/system-info.tssrc/lib/updater.ts
…ings - Fix performance service regex to match both MAXN and MAXN_SUPER - Extract duplicated disk fallback object into UNKNOWN_DISK constant - Fix WiFi warning: hotspot stops permanently, not temporarily - Add warning when hotspot is enabled that WiFi will disconnect Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
config/clawbox-performance.service (1)
8-8:⚠️ Potential issue | 🟠 MajorMatch both
MAXNandMAXN_SUPERwhen derivingMAXN_ID(Line 8).The current lookahead only matches
NAME=MAXN, soMAXN_SUPERentries are ignored and boot-time mode selection can be lower than intended.Suggested fix
-ExecStart=/bin/bash -c 'MAXN_ID=$(grep -oP "POWER_MODEL ID=\\K\\d+(?=\\s+NAME=MAXN)" /etc/nvpmodel.conf | tail -1); nvpmodel -m ${MAXN_ID:-0} && jetson_clocks' +ExecStart=/bin/bash -c 'MAXN_ID=$(grep -oP "POWER_MODEL ID=\\K\\d+(?=\\s+NAME=MAXN(?:_SUPER)?)" /etc/nvpmodel.conf | tail -1); nvpmodel -m ${MAXN_ID:-0} && jetson_clocks'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/clawbox-performance.service` at line 8, The grep in the ExecStart command's MAXN_ID extraction only matches NAME=MAXN and misses NAME=MAXN_SUPER; update the regex in that command (the ExecStart bash one that sets MAXN_ID) to look ahead for either NAME=MAXN or NAME=MAXN_SUPER (for example use a lookahead like (?=\s+NAME=MAXN(?:_SUPER)?) so entries labeled MAXN_SUPER are included), then retain the rest of the command (nvpmodel -m ${MAXN_ID:-0} && jetson_clocks).
🤖 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/components/DoneStep.tsx`:
- Around line 1328-1331: In the DoneStep component's JSX paragraph
(DoneStep.tsx, the <p> containing the note text), fix the unescaped apostrophe
by replacing the raw "You'll" with an escaped entity or a JS string literal
(e.g. use "You'll" or {"You'll"}) so the text no longer triggers
react/no-unescaped-entities; update the text inside the <p> that currently reads
"You'll need to reach..." accordingly.
- Around line 1555-1579: The hotspot SSID input (hotspotName) is disabled when
hotspotEnabled is false but saveSecurity still enforces a non-empty SSID, making
the form unsavable; update the validation in saveSecurity to only require a
non-empty hotspotName when hotspotEnabled is true (or alternatively clear
hotspotName when toggling hotspotEnabled off) so users can save the disabled
hotspot state; reference the hotspotEnabled and hotspotName state and the
saveSecurity function to locate and change the conditional validation logic.
---
Duplicate comments:
In `@config/clawbox-performance.service`:
- Line 8: The grep in the ExecStart command's MAXN_ID extraction only matches
NAME=MAXN and misses NAME=MAXN_SUPER; update the regex in that command (the
ExecStart bash one that sets MAXN_ID) to look ahead for either NAME=MAXN or
NAME=MAXN_SUPER (for example use a lookahead like (?=\s+NAME=MAXN(?:_SUPER)?) so
entries labeled MAXN_SUPER are included), then retain the rest of the command
(nvpmodel -m ${MAXN_ID:-0} && jetson_clocks).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 914a08e4-cdf1-4f2e-9726-fceeb0d3bb05
📒 Files selected for processing (3)
config/clawbox-performance.servicesrc/components/DoneStep.tsxsrc/lib/system-info.ts
- Escape apostrophe in JSX to avoid react/no-unescaped-entities - Only require hotspot name when hotspot is enabled - Update hotspot enabled warning to mention Ethernet access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Set hotspot_enabled=false when WiFi connects successfully - Update hotspot enabled warning to mention Ethernet/clawbox.local access Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
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/wifi/connect/route.ts (1)
29-37:⚠️ Potential issue | 🟠 MajorKeep
hotspot_enabledin sync on failure.Line 29 disables hotspot on success, but Line 37 only resets
wifi_configuredin the error path. Given Line 36 says AP is restored on failure, config can drift (hotspot_enabled: falsewhile AP is active).Suggested fix
- await set("wifi_configured", false).catch(() => {}); + await setMany({ wifi_configured: false, hotspot_enabled: true }).catch(() => {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/setup-api/wifi/connect/route.ts` around lines 29 - 37, The error path only resets wifi_configured but leaves hotspot_enabled possibly set to false; update the catch block in route.ts to also restore hotspot state by setting hotspot_enabled back to true (use setMany({ wifi_configured: false, hotspot_enabled: true }) or call set("hotspot_enabled", true) alongside set("wifi_configured", false)), and preserve the existing .catch(() => {}) to ignore any secondary errors; reference setMany, set, wifi_configured, and hotspot_enabled to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/app/setup-api/wifi/connect/route.ts`:
- Around line 29-37: The error path only resets wifi_configured but leaves
hotspot_enabled possibly set to false; update the catch block in route.ts to
also restore hotspot state by setting hotspot_enabled back to true (use
setMany({ wifi_configured: false, hotspot_enabled: true }) or call
set("hotspot_enabled", true) alongside set("wifi_configured", false)), and
preserve the existing .catch(() => {}) to ignore any secondary errors; reference
setMany, set, wifi_configured, and hotspot_enabled to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f65bf63c-42ad-4780-83d9-006a5ae3b92d
📒 Files selected for processing (1)
src/app/setup-api/wifi/connect/route.ts
…spot on connect - WiFi section shows DONE when wifi_configured is true (even if skipped in wizard) - Set hotspot_enabled=false on successful WiFi connect Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/DoneStep.tsx (1)
1055-1059:⚠️ Potential issue | 🟠 MajorSync
hotspotEnabledafter WiFi connect to prevent accidental hotspot re-enable.After a successful WiFi connect, backend flow disables hotspot, but local
hotspotEnabledis not updated. A subsequent Security save can post staleenabled: true(Line 682) and undo the WiFi-connect behavior.Proposed fix
setWifiStatus({ type: "success", message: "Connected!" }); setWifiConnectedSSID(wifiSSID.trim()); + setWifiDone(true); + setHotspotEnabled(false); setWifiSSID(""); setWifiPassword(""); setTimeout(() => { setOpenSection(null); setWifiStatus(null); }, 1500);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DoneStep.tsx` around lines 1055 - 1059, The successful WiFi connect handler updates UI state but doesn't sync the local hotspot flag, allowing a later save to re-enable hotspot; after the success sequence in the block that calls setWifiStatus, setWifiConnectedSSID, setWifiSSID, setWifiPassword and setOpenSection, also call the hotspot state updater (e.g., setHotspotEnabled(false) or the equivalent state setter used in this component) to set hotspotEnabled to false so subsequent Security saves use the correct value and won't accidentally re-enable the hotspot.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/DoneStep.tsx`:
- Around line 1055-1059: The successful WiFi connect handler updates UI state
but doesn't sync the local hotspot flag, allowing a later save to re-enable
hotspot; after the success sequence in the block that calls setWifiStatus,
setWifiConnectedSSID, setWifiSSID, setWifiPassword and setOpenSection, also call
the hotspot state updater (e.g., setHotspotEnabled(false) or the equivalent
state setter used in this component) to set hotspotEnabled to false so
subsequent Security saves use the correct value and won't accidentally re-enable
the hotspot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f34e29fb-d265-48f1-94bb-99f98155a620
📒 Files selected for processing (1)
src/components/DoneStep.tsx
- Send skip flag to wifi/connect when user clicks Skip (Ethernet only) - Handle skip in connect route: set wifi_configured without connecting - Dashboard WiFi section shows DONE for skipped WiFi Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merged
14 tasks
14 tasks
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.
…ovements
Summary by CodeRabbit