fix: state the browser integration honestly on the edition that has no switch - #383
Conversation
…o switch The Browser app's second step offered "Activate", and on a device with no OpenClaw CLI that button could only ever fail: the action wrote `tools.profile: full` and `tools.web.search.enabled` through the openclaw binary, so the owner got "The OpenClaw CLI is not available on this edition." next to a red "Fix the error" prompt — for a capability that was already working. There is nothing to activate there. The four ClawBox browser tools (browser_open / browser_navigate / browser_screenshot / browser_close) are registered on that edition unconditionally, scripts/register-mcp.sh wires the ClawBox MCP server into the harness config at every web-server boot, and it turns the harness's own browser toolset off so browsing goes through those tools and therefore through the Chromium window on the desktop. No tools profile to flip, no separate web-search tool to arm. So the route now answers `alwaysOn` and the panel renders that state: step 2 reads as already satisfied, names the tools the agent actually holds instead of an OpenClaw config key, and carries no button — a control with nothing to control is worse than no control at all. `enable` and `disable` are guarded server-side too, so a stale page or a direct call gets a plain answer rather than a CLI that isn't installed. Keyed on the edition rather than the active harness, so a dual device — which does have the CLI and the gateway — keeps the switch exactly as it was. The two hand-rolled `systemctl restart clawbox-gateway` calls now go through the existing restartGateway() helper, which already knows which editions have a gateway to bounce. Step 3 checks for Chromium itself rather than inheriting that check from step 2, which no longer implies it. New string in all ten locales; success toasts now name the running harness instead of hardcoding OpenClaw.
📝 WalkthroughWalkthroughThe browser integration now detects always-on editions, skips OpenClaw configuration for them, rejects disable requests, and uses ChangesBrowser integration behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant BrowserApp
participant BrowserManageRoute
participant EditionDetection
participant OpenClawConfig
BrowserApp->>BrowserManageRoute: Request browser status
BrowserManageRoute->>EditionDetection: Check integrationIsAlwaysOn()
alt Always-on edition
EditionDetection-->>BrowserManageRoute: alwaysOn = true
BrowserManageRoute-->>BrowserApp: Enabled built-in browser status
else OpenClaw edition
EditionDetection-->>BrowserManageRoute: alwaysOn = false
BrowserManageRoute->>OpenClawConfig: Read integration configuration
OpenClawConfig-->>BrowserManageRoute: Configuration state
BrowserManageRoute-->>BrowserApp: Switch-controlled status
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
🦀 ClawReviewYour friendly reef crab, here with the lay of the land. On Hermes-edition devices (no OpenClaw CLI installed), the Browser app's step 2 previously showed an Activate button that could only produce an error, because the underlying action reached for a binary that isn't there. This PR fixes that by teaching the route to detect the always-on edition via At a glance
Good to know
— ClawReview 🦀, scuttling off. General info only — see CodeRabbit for the detailed review. Conventions: docs. |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/BrowserApp.tsx`:
- Around line 267-269: Update the alwaysOn label in BrowserApp to include
browser_close alongside the other available browser tools, while leaving the
non-always-on “tools profile: full” label unchanged.
In `@src/tests/routes/browser/manage.test.ts`:
- Around line 261-264: Update the systemctlCalls filter in the relevant browser
management test to match the exact gateway service argument,
“clawbox-gateway.service”, so direct restart calls are detected while preserving
the existing zero-call assertion.
🪄 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: 93aa3241-55fb-4db3-93f6-ec820f915280
📒 Files selected for processing (8)
src/app/setup-api/browser/manage/route.tssrc/components/BrowserApp.tsxsrc/lib/desktop-translations-part1.tssrc/lib/desktop-translations-part2.tssrc/lib/desktop-translations-part3.tssrc/lib/desktop-translations.tssrc/tests/components/browser-app.test.tsxsrc/tests/routes/browser/manage.test.ts
| <span className={`text-xs text-white/40${alwaysOn ? " font-mono" : ""}`}> | ||
| {alwaysOn ? "browser_open · browser_navigate · browser_screenshot" : "tools profile: full"} | ||
| </span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
List every available browser tool.
The always-on label omits browser_close. The route describes it as an available browser tool. The panel therefore reports an incomplete capability set.
Proposed fix
- {alwaysOn ? "browser_open · browser_navigate · browser_screenshot" : "tools profile: full"}
+ {alwaysOn ? "browser_open · browser_navigate · browser_screenshot · browser_close" : "tools profile: full"}📝 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.
| <span className={`text-xs text-white/40${alwaysOn ? " font-mono" : ""}`}> | |
| {alwaysOn ? "browser_open · browser_navigate · browser_screenshot" : "tools profile: full"} | |
| </span> | |
| <span className={`text-xs text-white/40${alwaysOn ? " font-mono" : ""}`}> | |
| {alwaysOn ? "browser_open · browser_navigate · browser_screenshot · browser_close" : "tools profile: full"} | |
| </span> |
🤖 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/BrowserApp.tsx` around lines 267 - 269, Update the alwaysOn
label in BrowserApp to include browser_close alongside the other available
browser tools, while leaving the non-always-on “tools profile: full” label
unchanged.
| const systemctlCalls = mockExec.mock.calls.filter( | ||
| ([, args]) => Array.isArray(args) && args.includes("clawbox-gateway"), | ||
| ); | ||
| expect(systemctlCalls).toHaveLength(0); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the gateway service argument exactly.
args.includes("clawbox-gateway") does not match the direct service argument, clawbox-gateway.service. The test can pass if a direct systemctl restart clawbox-gateway.service call returns.
Proposed fix
- ([, args]) => Array.isArray(args) && args.includes("clawbox-gateway"),
+ ([, args]) => Array.isArray(args) && args.includes("clawbox-gateway.service"),📝 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.
| const systemctlCalls = mockExec.mock.calls.filter( | |
| ([, args]) => Array.isArray(args) && args.includes("clawbox-gateway"), | |
| ); | |
| expect(systemctlCalls).toHaveLength(0); | |
| const systemctlCalls = mockExec.mock.calls.filter( | |
| ([, args]) => | |
| Array.isArray(args) && args.includes("clawbox-gateway.service"), | |
| ); | |
| expect(systemctlCalls).toHaveLength(0); |
🤖 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/tests/routes/browser/manage.test.ts` around lines 261 - 264, Update the
systemctlCalls filter in the relevant browser management test to match the exact
gateway service argument, “clawbox-gateway.service”, so direct restart calls are
detected while preserving the existing zero-call assertion.
1 participant
The bug
On a device with no OpenClaw CLI, the Browser app's second step offered an Activate button that could only ever fail. The action wrote
tools.profile: fullandtools.web.search.enabled: truethrough theopenclawbinary, so the owner got a red banner —— next to a "Fix the error" prompt that would have sent the on-device agent hunting through journald for a fault that does not exist. Step 1 (Chromium) and step 3 (open the desktop browser) both worked; only the step in between was broken, and it was broken by claiming a capability needed setting up when it was already in place.
Why there is nothing to activate there
Browsing already works on that edition, by design and at every boot:
mcp/tools/browser.tsregistersbrowser_open/browser_navigate/browser_screenshot/browser_closefor both editions — only the four coordinate tools are OpenClaw-only. They drive the same desktop Chromium over CDP 18800.scripts/register-mcp.shwires the ClawBox MCP server into the harness config at every web-server boot, then turns the harness's own browser toolset off, logging "built-in browser toolset off; browsing goes through the ClawBox browser_ tools."* That is deliberate: the built-in one drives a browser the owner cannot see, and its engine is not provisioned on this image.tools.profileon that harness to flip, and no separate web-search tool to arm — browsing is the web mechanism.A per-user toggle would also fight the boot-time reconciliation that script exists to perform, so a "real" Hermes activation was rejected rather than invented.
What changed
src/app/setup-api/browser/manage/route.tsGETanswers a newalwaysOnflag, and reportsenabled: trueon that edition. It skips the OpenClaw config read and the sqlite flag entirely there — neither describes a device that keeps no such files.enableshort-circuits after creating the browser profile dir, returning{ ok: true, enabled: true, alwaysOn: true }without touching the CLI. It still refuses when Chromium is missing.disableanswers plainly that there is nothing to turn off, rather than reporting a success that changed nothing.sudo systemctl restart clawbox-gatewaycalls now go through the existingrestartGateway()helper, which already knows which editions have a gateway to bounce (and has the user-unit fallback).src/components/BrowserApp.tsxalwaysOn: satisfied state, no button, and copy that says browsing is built in.ErrorWithFixtherefore never fires here.tools profile: full, which is an OpenClaw config key that does not exist on the other edition.Translations — one new key,
browser.builtInMessage, in all ten locales.Not a regression for OpenClaw or dual
The branch is keyed on the edition (
openclawIsAbsent()), not the active harness. Adualdevice still ships the CLI and the gateway, so it keeps the switch, the config writes and the gateway bounce exactly as before. A test asserts the switch shape is still reported on an edition that ships the CLI, and thatrunOpenclawConfigSetis still called there.Tests
16 new tests, all failing without the change (verified by mutation: forcing the flag off fails 5 route tests and 3 panel tests).
src/tests/routes/browser/manage.test.ts— GET reports on/alwaysOn; GET reads no OpenClaw config; GET stays on even if the old switch was persisted off;enablesucceeds with no CLI call and no gateway bounce;enablestill refuses without Chromium;disableanswers without the CLI and without writing sqlite; the desktop browser controls are untouched; the gateway bounce goes through the shared helper; and the switch edition still reportsalwaysOn: false.src/tests/components/browser-app.test.tsx— panel renders the toggle, connected copy and tools-profile chip on a switch device; renders no toggle, built-in copy and the tool-name chip on an always-on device, and still offers the desktop browser controls.Full suite: 2383 passed, same 32 pre-existing failures as
betaon this Windows dev host (path-separator and file-mode assertions). Typecheck and lint clean on every changed file.Still to verify on hardware
I could not reach the Hermes device to deploy or check the panel. SSH on
192.168.50.190rejects both the documented password and key auth (Permission denied (publickey,password)), and the device password rotates at each setup. The reasoning above is settled by the code, but the live confirmations are still owed:hermes mcp list/hermes tools list— clawbox server present, built-in browser toolset disabled;browser_openfrom a chat turn and watch the desktop window respond.Happy to run all three as soon as I have the current credential for that box.
Summary by CodeRabbit
New Features
Bug Fixes