Skip to content

feat: Hermes single-harness edition + premium dual, with dashboard SSO - #361

Merged
KrasimirKralev merged 79 commits into
betafrom
feat/hermes-edition
Aug 11, 2026
Merged

feat: Hermes single-harness edition + premium dual, with dashboard SSO#361
KrasimirKralev merged 79 commits into
betafrom
feat/hermes-edition

Conversation

@KrasimirKralev

@KrasimirKralev KrasimirKralev commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Hermes edition (single-harness) + dual as a premium feature

Adds a single-harness edition model so a device ships locked to one agent
harness and a customer can't freely switch:

  • CLAWBOX_EDITION = openclaw | hermes | dual, baked at install time into
    a root-owned systemd drop-in (not user-editable config), selected via env
    or config/edition.txt. Default = openclaw (the native product: single,
    locked). hermes is its own SKU.
  • dual (both harnesses + an in-UI switcher) is a premium feature gated
    by a signed license; without a valid license a dual image degrades to the
    locked native edition. The signing key lives off-device/off-repo.
  • Runtime lock in src/lib/harness.ts (single editions force the active harness
    and reject switch attempts); /harness/select → 403 when locked;
    /harness/status advertises only the active harness; the desktop hides the
    other harness's app.

Hermes edition specifics

  • install.sh is edition-aware: on hermes it skips the OpenClaw gateway steps,
    installs the Hermes agent, and stands up the dashboard + auth-proxy services.
  • Setup wizard adapts (Hermes manages its own models; Anthropic subscription
    OAuth surfaced).
  • Hermes dashboard desktop icon: hermes dashboard runs host-local; an
    isolated, clawbox_session-gated reverse proxy fronts it on a dedicated LAN
    port and signs the already-authenticated ClawBox user in transparently, so
    there's no second login and its WebSocket handshake works end-to-end.
  • Chat popup model picker for Hermes (see and switch the provider/model,
    mirroring the OpenClaw chat).

Tests green (1725). Verified live on-device.

Summary by CodeRabbit

  • New Features

    • Added support for OpenClaw, Hermes, and licensed dual-harness editions.
    • Added the Hermes dashboard with secure sign-in, automatic startup, and LAN access.
    • Added Hermes model discovery, selection, chat model picking, and provider configuration.
    • Added Claude Pro/Max subscription sign-in through OAuth.
    • Added a Hermes Skills Store for discovering, installing, and uninstalling skills.
    • Added signed dual-edition license validation and expiration handling.
  • Enhancements

    • Single-harness editions now show the active edition and prevent switching.
    • Improved app visibility and layout based on the active harness.
    • Added fallback handling for unavailable model catalogs.

KrasimirKralev and others added 7 commits August 10, 2026 12:35
…law, add hermes services) + config units + setup script
…fault = openclaw + hermes installer + wizard hermes panel
… show full model id in chat picker + dashboard port-contention guard
The Hermes dashboard only engages its cookie-session auth gate on a
non-loopback bind; on plain 127.0.0.1 it falls back to a legacy
shared-token mode that ignores session cookies, so the reverse proxy
could never authenticate and the SPA's WebSocket handshake failed
(code 1006). Bind the dashboard to 127.0.0.2 (still host-local, never
LAN-reachable) so gated cookie mode engages, configure the bundled
password provider, and have the proxy log in with a server-side-only
password and inject/relay the Hermes session so the ClawBox user is
signed in transparently (no second login). WS upgrades now return 101.
@KrasimirKralev
KrasimirKralev requested a review from a team as a code owner August 10, 2026 12:48
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This change adds Hermes edition installation and service management, authenticated dashboard proxying, dual-edition licensing, harness locking, Skills Store APIs, model configuration, and harness-aware dashboard and chat behavior.

Changes

Hermes edition support

Layer / File(s) Summary
Edition licensing and harness locking
src/lib/edition-license.ts, src/lib/harness.ts, scripts/issue-dual-license.mjs, src/tests/unit/harness-edition.test.ts
Adds signed license verification, edition resolution, locked harness behavior, dual-edition access checks, license issuance, and unit coverage.
Hermes installation and service setup
install.sh, scripts/setup-hermes-dashboard-auth.sh, scripts/setup-hermes-edition.sh, config/clawbox-hermes-dashboard.service, config/clawbox-hermes-dashboard-proxy.service
Adds Hermes installation, edition locking, dashboard authentication setup, systemd unit installation, and service startup.
Authenticated dashboard proxy
scripts/hermes-dashboard-proxy.js
Adds ClawBox session validation, Hermes login brokering, cookie handling, HTTP retry logic, request-body buffering, and WebSocket proxying.
Harness status and Hermes configuration APIs
src/app/setup-api/harness/*, src/app/setup-api/hermes/models/route.ts, src/app/setup-api/hermes/provider-key/route.ts, src/lib/hermes-cli.ts
Rejects harness changes on locked devices, reports edition metadata, and exposes validated Hermes model and provider-key configuration through the Hermes CLI.
Hermes Skills Store
src/lib/hermes-skills.ts, src/lib/hermes-skills-server.ts, src/app/setup-api/hermes/skills/*, src/components/HermesSkillsStore.tsx
Adds skill validation, installed-skill enumeration, search, installation, uninstallation, and the Skills Store interface.
Harness-aware dashboard and chat UI
src/app/page.tsx, src/components/AIModelsStep.tsx, src/components/ChatPopup.tsx, src/components/HarnessPicker.tsx, src/components/HermesProviderConfig.tsx
Hides inactive harness apps, displays locked status, opens the Hermes proxy, supports Hermes model selection, and provides Hermes provider configuration.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant ClawBoxUI
  participant DashboardProxy
  participant HermesDashboard
  User->>ClawBoxUI: Open Hermes dashboard
  ClawBoxUI->>DashboardProxy: Request current host on port 8090
  DashboardProxy->>DashboardProxy: Validate ClawBox session
  DashboardProxy->>HermesDashboard: Authenticate and forward request
  HermesDashboard-->>DashboardProxy: Return dashboard response
  DashboardProxy-->>ClawBoxUI: Return proxied response
Loading

Possibly related PRs

Suggested labels: enhancement

Suggested reviewers: georgik77

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides a detailed summary and test result, but it omits the required change type, test checklist, repository checklist, and screenshots or logs section. Add the missing template sections and record change type, exact test commands, checklist status, and screenshots or logs where applicable.
Docstring Coverage ⚠️ Warning Docstring coverage is 51.28% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: Hermes single-harness support, premium dual-harness support, and dashboard SSO.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/hermes-edition

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦀 ClawReview

Fresh PR washed in with the tide — here's the gist.

This PR introduces a first-class edition model for ClawBox devices: a locked single-harness "Hermes" SKU, the existing "openclaw" default, and a premium "dual" mode (both harnesses + UI switcher) gated by a signed off-device license. Alongside the edition system it ships the Hermes dashboard as a proper desktop app — fronted by a dedicated auth proxy so the already-authenticated ClawBox session carries over transparently, no second login. The MCP server was also substantially restructured: the monolithic clawbox-mcp.ts was split into focused tool modules under mcp/tools/ and shared utilities under mcp/lib/.

At a glance

  • ✨ Feature · touches edition system, install.sh, MCP server, Hermes dashboard proxy + SSO, setup wizard, desktop environment, systemd services
  • Base branch: beta · +22327 source / +5766 tests across 207 files
  • ✅ base beta matches the beta-first convention
  • ✅ conventional PR title
  • ✅ source changes come with test changes
  • 🟡 large PR (33724 lines changed) — consider splitting
  • ℹ️ touches security-sensitive paths (config/clawbox-gateway.service, config/clawbox-hermes-dashboard-proxy.service, config/clawbox-hermes-dashboard.service, config/clawbox-root-update@.service, …) — review with extra care

Good to know

  • 🟡 207 files and ~33 k lines changed — touches install, systemd, MCP, setup wizard, desktop, and OAuth all in one go; careful merge-conflict watch on the beta branch is worthwhile.
  • 🟡 Adds new systemd services, a sudoers entry, and root-owned drop-ins for edition locking — these land directly on customer hardware at install time, so the install.sh code paths deserve a close read.
  • ℹ️ New package.json entries in both the root and mcp/ packages; bun.lock will update on next install — worth a quick scan to confirm no unexpected transitive additions.
  • ℹ️ The signed dual-edition license mechanism (scripts/issue-dual-license.mjs) keeps the signing key off-device and off-repo, which is intentional per the PR description — just good to flag for anyone unfamiliar with the licensing model.

— ClawReview 🦀. I set the scene; CodeRabbit reviews the code; you decide. Conventions: docs.

@github-actions github-actions Bot added area: install Auto-triage area area: ui Auto-triage area labels Aug 10, 2026
@github-actions

github-actions Bot commented Aug 10, 2026

Copy link
Copy Markdown

CI Summary

✅ Tests

  • Result: passed
  • View run
  • Coverage: statements 64.26%, branches 53.26%, functions 61.86%, lines 66.35%

✅ E2E

❌ E2E Install

Comment thread scripts/hermes-dashboard-proxy.js Dismissed
},
);
reqUp.on("error", (e) => {
console.error(`[hermes-dashboard-proxy] login error: ${e.message}`);
Comment thread scripts/hermes-dashboard-proxy.js Fixed

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 29

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/components/ChatPopup.tsx (1)

1048-1051: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Abort the Hermes request during component cleanup.

hermesAbortRef is aborted only by the Stop action. If the page unmounts while a Hermes request runs, the fetch and its Hermes process continue until completion. The completion handler can also update unmounted component state.

Abort hermesAbortRef.current in the existing unmount cleanup.

Proposed fix
 return () => {
+  hermesAbortRef.current?.abort()
   wsRef.current?.close()
   wsRef.current = null
🤖 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/ChatPopup.tsx` around lines 1048 - 1051, Abort the active
Hermes request during component unmount by updating the existing cleanup effect
to call abort on hermesAbortRef.current when present. Keep the existing Stop
action behavior and ensure cleanup safely handles a null ref.
🤖 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 `@config/clawbox-hermes-dashboard-proxy.service`:
- Around line 3-4: Update the Wants= declaration in the proxy service unit to
include clawbox-setup.service alongside clawbox-hermes-dashboard.service,
preserving the existing After= ordering so the proxy explicitly pulls in both
required services.
- Around line 6-15: Add systemd sandboxing to both units:
config/clawbox-hermes-dashboard-proxy.service lines 6-15 and
config/clawbox-hermes-dashboard.service lines 6-26. Configure NoNewPrivileges,
ProtectSystem, ProtectHome, and PrivateTmp; for the proxy, permit read-only
access only to data/.session-secret, data/.hermes-dashboard-pw, and
data/config.json while keeping the rest of home read-only or appropriately
bound. For the dashboard service, grant ReadWritePaths only for its required
runtime and web-dist build directories, not the entire home directory.

In `@config/clawbox-hermes-dashboard.service`:
- Around line 1-4: Add explicit systemd restart-rate limiting in the [Unit]
section of clawbox-hermes-dashboard.service using appropriate
StartLimitIntervalSec and StartLimitBurst values, while preserving the existing
restart behavior; if permanent retries are intentionally required, explicitly
set StartLimitBurst=0 instead.

In `@install.sh`:
- Around line 794-799: Update step_hermes_install to resolve a single Hermes
executable path using the HERMES_BIN override with
$CLAWBOX_HOME/.local/bin/hermes as the fallback, then use that resolved variable
for both the executable check and version logging. Keep the install flow aligned
with the path consumed by the runtime and setup script.
- Around line 810-820: The drop-in has two writers, allowing execution order to
override the validated edition. In install.sh:810-820, keep step_edition_lock as
the sole writer and preserve its configured CLAWBOX_EDITION value. In
scripts/setup-hermes-edition.sh:58-65, remove the direct edition.conf write and
reuse the shared writer or helper so both callers produce the drop-in from the
same configured value.
- Around line 810-820: Remove the duplicate edition.conf write from
step_edition_lock, including its directory creation and related
daemon-reload/logging that only support that write, so
scripts/setup-hermes-edition.sh remains the single writer of the systemd drop-in
while preserving the edition validation behavior.
- Around line 130-142: Add clawbox-hermes-dashboard.service and
clawbox-hermes-dashboard-proxy.service to EXPECTED_INSTALLED_SERVICES so
step_systemd_services registers both units for every edition. Keep them excluded
from EXPECTED_ACTIVE_SERVICES in the is_hermes_edition branch, allowing the
Hermes setup script to install and enable them.
- Around line 800-806: Update the Hermes installation block to download the
remote installer into a temporary file, verify it against a pinned SHA-256
checksum or trusted signature, and execute it only after successful integrity
validation. Keep the command running as CLAWBOX_USER, avoid piping the response
directly into bash, and retain the non-fatal warning only for failures after
verification is enforced.

In `@scripts/hermes-dashboard-proxy.js`:
- Around line 248-252: Update the browserHasHermesSession pass-through flow to
detect an upstream 401 response and retry using the existing SSO injection path
instead of leaving the client at the login page. Preserve direct forwarding for
successful responses and keep the existing hasValidSession authorization gate
unchanged.
- Around line 119-161: Add bounded socket timeouts to all upstream connections:
configure the HTTP request created in hermesLogin with reqUp.setTimeout, apply
upstream.setTimeout in forward, and configure the net.Socket returned by the
WebSocket upgrade handler’s net.connect. On timeout, destroy the affected socket
and follow each existing failure/cleanup path so hung clients and upstream
connections are released.
- Around line 163-174: Update ensureHermesCookies so forceRefresh cannot reuse
an existing loginInFlight promise: when refresh is requested during an in-flight
login, chain or otherwise schedule a new hermesLogin after the current attempt
completes, while preserving de-duplication for non-refresh callers and updating
hermesState/loginInFlight consistently.
- Around line 200-214: Update the 401 retry condition in forward to use the
existing replayability signal (!tooBig) instead of requiring bodyBuf !== null,
so GET and HEAD requests can re-authenticate and retry. Keep bodyBuf handling
unchanged for request transmission, and preserve the existing single-retry
behavior.
- Around line 193-201: Update forward and pipeResponse to remove hop-by-hop
headers (Connection, Keep-Alive, Proxy-*, TE, Trailer, Transfer-Encoding, and
Upgrade) before forwarding request or response headers; when bodyBuf sets
Content-Length, ensure Transfer-Encoding is removed to prevent conflicting
framing, and filter upstream headers before pipeResponse calls res.writeHead.

In `@scripts/issue-dual-license.mjs`:
- Around line 26-33: Validate the --days argument before constructing the
payload in the argument-handling flow, rejecting present values that are not
finite numeric day counts instead of generating exp: NaN. Preserve omission of
exp when --days is absent. Also update the verifier’s expiry handling in the
symbol containing the payload.exp check so a present non-numeric exp invalidates
the license rather than being treated as an absent expiry.

In `@scripts/setup-hermes-dashboard-auth.sh`:
- Line 76: Update the generated YAML in the setup script’s username output to
quote the USERNAME value, ensuring values containing YAML-special characters or
a leading hyphen remain valid and are parsed as a string by Hermes.
- Around line 24-28: Update the setup script’s idempotency flow to detect an
existing dashboard configuration block before generating credentials, while
preserving the existing missing-config initialization check. If the config
already contains dashboard settings, skip password generation and credential
writes regardless of the password file state; otherwise continue with normal
generation and configuration creation.

In `@scripts/setup-hermes-edition.sh`:
- Around line 58-65: Remove the duplicate edition drop-in write from
scripts/setup-hermes-edition.sh and rely on the existing step_edition_lock
implementation in install.sh as the single writer. Keep the Hermes setup flow’s
edition selection intact without creating or overwriting the systemd drop-in in
both locations.
- Around line 11-15: Make the getent lookup in the CLAWBOX_HOME assignment
non-fatal under set -euo pipefail, allowing the existing /home/$CLAWBOX_USER
fallback to execute when no passwd entry exists. Preserve the current
CLAWBOX_HOME and HERMES_BIN behavior for successful lookups.
- Around line 67-74: Update the service restart block in
scripts/setup-hermes-edition.sh to track whether any systemctl restart fails,
including clawbox-hermes-dashboard.service,
clawbox-hermes-dashboard-proxy.service, and clawbox-setup.service. Keep logging
each failure, then exit with a non-zero status after the completion log when the
failure flag is set, while preserving a zero exit for successful restarts.

In `@src/app/page.tsx`:
- Around line 254-264: Update the active harness state and harnessHiddenAppId
logic near activeHarness so it remains unknown until /setup-api/harness/active
returns a valid harness, rather than defaulting to "openclaw". While the request
is pending or fails, hide both harness-specific apps, and add the requested
retry or explicit unavailable handling for failed requests.
- Around line 993-1001: Update the desktop context-menu rendering to derive the
harness action from the filtered app list produced by getAllApps(), rather than
always rendering OpenClaw. Show the active harness’s app action when present and
omit it when no matching app exists, preserving the existing behavior for other
context-menu actions.

In `@src/app/setup-api/hermes/models/route.ts`:
- Line 70: Update the current declaration in the route model setup to use const
instead of let, since current is never reassigned. Preserve the existing
fallback chain unchanged.
- Around line 56-57: Update the config-default parsing logic around the
raw.match expression so it reads only the value of model.default, not the first
default or model key anywhere in config.yaml. Prefer the existing YAML parsing
mechanism if available; otherwise constrain matching to the model block and
preserve MODEL_ID_RE validation before returning the ID.

In `@src/components/AIModelsStep.tsx`:
- Around line 544-552: Update the edition state and status-fetch handling in the
AIModelsStep component so failed or invalid responses remain unresolved instead
of defaulting to "openclaw"; render a loading or retry state while edition is
unknown, and only show the OpenClaw form after a successful response explicitly
identifies that edition.

In `@src/components/HarnessPicker.tsx`:
- Around line 83-87: The locked badge in HarnessPicker’s active harness display
currently hardcodes the status dot as green. Use the active harness entry from
status.harnesses, identified by status.active, and derive the dot color from its
healthy value while preserving the existing healthy appearance and applying an
unhealthy appearance when healthy is false.

In `@src/lib/edition-license.ts`:
- Around line 13-16: Update the documentation to reflect that dual-license
enforcement is active: in src/lib/edition-license.ts lines 13-16, replace the
pre-key/empty-key description with one stating that the embedded key enforces
licensing; in src/lib/harness.ts lines 48-58, remove the statement that dual
remains open and state that dual requires a valid license.
- Around line 45-55: Memoize the license verdict returned by verifyDualLicense()
so readLicenseString() and Ed25519 verification are performed only once per
process lifetime. Add a cached result in the edition-license module and have
subsequent isDualUnlocked() calls reuse it, preserving the existing verdict for
both valid and invalid or missing licenses.
- Line 30: Remove the unpinned CLAWBOX_LICENSE_PUBKEY environment override from
the DUAL_LICENSE_PUBKEY configuration so verifyDualLicense() always uses the
compiled-in EMBEDDED_DUAL_LICENSE_PUBKEY; alternatively, validate any override
against a compiled-in public-key fingerprint allowlist before accepting it.
Preserve isDualLicenseEnforced() and signature verification behavior for the
approved key.

In `@src/tests/unit/harness-edition.test.ts`:
- Around line 14-17: Update the harness edition tests to make the mocked
isDualLicenseEnforced flag mutable, then add coverage verifying isDualUnlocked()
grants access when enforcement is false. Strengthen the existing rejection
assertions with the exact message "Harness switching is disabled on this
edition", and assert mockSet was not called after each rejected setActiveHarness
attempt.

---

Outside diff comments:
In `@src/components/ChatPopup.tsx`:
- Around line 1048-1051: Abort the active Hermes request during component
unmount by updating the existing cleanup effect to call abort on
hermesAbortRef.current when present. Keep the existing Stop action behavior and
ensure cleanup safely handles a null ref.
🪄 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: 2eed961e-1502-492e-81f6-86364ac250c9

📥 Commits

Reviewing files that changed from the base of the PR and between 1b9158e and dddce8b.

⛔ Files ignored due to path filters (1)
  • public/hermes-agent.png is excluded by !**/*.png
📒 Files selected for processing (17)
  • config/clawbox-hermes-dashboard-proxy.service
  • config/clawbox-hermes-dashboard.service
  • install.sh
  • scripts/hermes-dashboard-proxy.js
  • scripts/issue-dual-license.mjs
  • scripts/setup-hermes-dashboard-auth.sh
  • scripts/setup-hermes-edition.sh
  • src/app/page.tsx
  • src/app/setup-api/harness/select/route.ts
  • src/app/setup-api/harness/status/route.ts
  • src/app/setup-api/hermes/models/route.ts
  • src/components/AIModelsStep.tsx
  • src/components/ChatPopup.tsx
  • src/components/HarnessPicker.tsx
  • src/lib/edition-license.ts
  • src/lib/harness.ts
  • src/tests/unit/harness-edition.test.ts

Comment on lines +3 to +4
After=clawbox-hermes-dashboard.service clawbox-setup.service
Wants=clawbox-hermes-dashboard.service

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Wants= omits clawbox-setup.service.

Line 3 orders the proxy after clawbox-setup.service, but Line 4 requests only the dashboard. Ordering without a dependency has no effect when clawbox-setup.service is not otherwise pulled into the transaction. The proxy reads data/.session-secret and data/config.json, both owned by the main app, so state the dependency explicitly.

♻️ Proposed change
 After=clawbox-hermes-dashboard.service clawbox-setup.service
-Wants=clawbox-hermes-dashboard.service
+Wants=clawbox-hermes-dashboard.service clawbox-setup.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.

Suggested change
After=clawbox-hermes-dashboard.service clawbox-setup.service
Wants=clawbox-hermes-dashboard.service
After=clawbox-hermes-dashboard.service clawbox-setup.service
Wants=clawbox-hermes-dashboard.service clawbox-setup.service
🤖 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 `@config/clawbox-hermes-dashboard-proxy.service` around lines 3 - 4, Update the
Wants= declaration in the proxy service unit to include clawbox-setup.service
alongside clawbox-hermes-dashboard.service, preserving the existing After=
ordering so the proxy explicitly pulls in both required services.

Source: Path instructions

Comment thread config/clawbox-hermes-dashboard-proxy.service
Comment on lines +1 to +4
[Unit]
Description=ClawBox Hermes Dashboard (host-local web UI, gated auth)
After=network-online.target
Wants=network-online.target

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Add restart rate limiting.

Restart=always with RestartSec=5 and no StartLimitIntervalSec/StartLimitBurst override uses the systemd defaults. On an embedded Jetson a crash loop that builds the web dist on each start consumes CPU continuously. Set an explicit limit, or set StartLimitBurst=0 deliberately if a permanent retry is intended.

🤖 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 `@config/clawbox-hermes-dashboard.service` around lines 1 - 4, Add explicit
systemd restart-rate limiting in the [Unit] section of
clawbox-hermes-dashboard.service using appropriate StartLimitIntervalSec and
StartLimitBurst values, while preserving the existing restart behavior; if
permanent retries are intentionally required, explicitly set StartLimitBurst=0
instead.

Source: Path instructions

Comment thread install.sh Outdated
Comment thread install.sh
Comment thread src/components/HarnessPicker.tsx Outdated
Comment thread src/lib/edition-license.ts Outdated
Comment thread src/lib/edition-license.ts Outdated
Comment on lines +45 to +55
function readLicenseString(): string | null {
const env = process.env.CLAWBOX_DUAL_LICENSE?.trim();
if (env) return env;
try {
const root = process.env.CLAWBOX_ROOT || "/home/clawbox/clawbox";
const raw = fs.readFileSync(path.join(root, "data", "dual-license.txt"), "utf8").trim();
return raw || null;
} catch {
return null;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache the license verdict. The current code performs a blocking read and an Ed25519 verification on every request path.

verifyDualLicense() calls readLicenseString(), which runs fs.readFileSync synchronously. isDualUnlocked() calls verifyDualLicense(), and getActiveHarness(), isSingleHarnessEdition(), and lockedHarness() in src/lib/harness.ts call isDualUnlocked(). On a dual device, every /setup-api/harness/status GET, every /setup-api/harness/select POST, and every render path that resolves the active harness therefore blocks the Node event loop on disk I/O and then runs a signature verification. The device is an embedded Jetson with limited CPU.

The license is static for the process lifetime, the same as DUAL_LICENSE_PUBKEY. Memoize the result.

♻️ Proposed memoization
+// The license and the key are fixed for the process lifetime, so verify once.
+// Re-reading the file on every harness lookup would put a blocking readFileSync
+// plus an ed25519 verification on the request path of an embedded device.
+let cachedVerdict: boolean | null = null;
+
 export function verifyDualLicense(): boolean {
+  if (cachedVerdict !== null) return cachedVerdict;
+  cachedVerdict = computeDualLicenseVerdict();
+  return cachedVerdict;
+}
+
+function computeDualLicenseVerdict(): boolean {
   if (!DUAL_LICENSE_PUBKEY) return false; // licensing not wired yet → locked

If a license installed after boot must take effect without a service restart, keep the read but bound it with a short time-to-live instead of caching forever.

As per path instructions for src/app/**/*.ts* and src/lib/**: "Resource constraints (limited memory/CPU on embedded hardware)".

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] 49-49: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(path.join(root, "data", "dual-license.txt"), "utf8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

🤖 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/edition-license.ts` around lines 45 - 55, Memoize the license verdict
returned by verifyDualLicense() so readLicenseString() and Ed25519 verification
are performed only once per process lifetime. Add a cached result in the
edition-license module and have subsequent isDualUnlocked() calls reuse it,
preserving the existing verdict for both valid and invalid or missing licenses.

Source: Path instructions

Comment thread src/tests/unit/harness-edition.test.ts
The dashboard's WebSocket Host/Origin guard rejects any upgrade whose
Origin doesn't target its bound host. The proxy rewrote Host but left the
browser's real Origin (the LAN proxy URL), so every dashboard WS —
including the chat events feed — closed before accept (code 1006 /
'connection failed'). Rewrite Origin and Referer to the upstream origin
on both HTTP and WS. Verified: /api/ws, /api/events, /api/pty, /api/console
all return 101 with a browser Origin header.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@scripts/hermes-dashboard-proxy.js`:
- Around line 49-52: Validate the request Origin against the configured trusted
UI origins before rewriting headers or forwarding state-changing HTTP requests
and WebSocket upgrades in the handlers around the session checks and proxy
forwarding paths. Reject untrusted and null origins, while allowing configured
UI origins; add coverage for both rejected cross-origin requests and accepted
configured origins.
🪄 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: 953fabad-34be-466e-ad1a-44ab0c9030ec

📥 Commits

Reviewing files that changed from the base of the PR and between dddce8b and 42b3d3a.

📒 Files selected for processing (1)
  • scripts/hermes-dashboard-proxy.js

Comment thread scripts/hermes-dashboard-proxy.js
Replace the static Hermes AI-provider panel with a real config surface:
pick the default model + inference provider (persisted via hermes config
set) and add a provider API key (hermes auth add). New shared runHermesCli
helper (argv-only, no shell), setter on /setup-api/hermes/models, and a
/setup-api/hermes/provider-key route with provider allowlist + key
validation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/hermes/models/route.ts (1)

73-84: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return the configured provider in the GET response.

The POST route persists model.provider, but this response returns only models and current. HermesProviderConfig initializes provider to "auto" and sends it with every save. A user who changes only the model can therefore overwrite an existing provider selection with "auto".

Read and return the configured provider. Initialize the client state from that value.

🤖 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/hermes/models/route.ts` around lines 73 - 84, Update GET to
read the configured provider alongside the configured default, include that
provider in the NextResponse.json payload, and ensure the client state
initializes from the returned configured provider instead of defaulting to
“auto”.
🤖 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/app/setup-api/hermes/models/route.ts`:
- Around line 91-135: Protect both setup API handlers before executing Hermes
commands: in src/app/setup-api/hermes/models/route.ts lines 91-135, require
authorization before either runHermesCli config set call; in
src/app/setup-api/hermes/provider-key/route.ts lines 22-57, require
authorization before invoking hermes auth add. Preserve the existing validation
and response behavior after authorization succeeds.

In `@src/app/setup-api/hermes/provider-key/route.ts`:
- Around line 41-44: Update the Hermes CLI invocation in the provider-key route
so apiKey is not included in the argument vector passed to runHermesCli. Use a
Hermes-supported protected credential channel such as stdin or a file
descriptor, while preserving the existing auth add operation and timeout
behavior.

In `@src/components/HermesProviderConfig.tsx`:
- Around line 193-201: Update the API-key input in HermesProviderConfig to
include a unique id and an associated visible or screen-reader-only label, while
preserving its existing password behavior and state handling.

In `@src/lib/hermes-cli.ts`:
- Around line 53-67: Update the child process handling around the stdout and
stderr data listeners to narrow each nullable stream before registering
listeners, and guard the input path so child.stdin is checked before calling
end. Preserve the existing output-limit, kill, and rejection behavior in the
narrowed branches.

---

Outside diff comments:
In `@src/app/setup-api/hermes/models/route.ts`:
- Around line 73-84: Update GET to read the configured provider alongside the
configured default, include that provider in the NextResponse.json payload, and
ensure the client state initializes from the returned configured provider
instead of defaulting to “auto”.
🪄 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: 8e1e7218-a2c6-4c1e-b3a7-c5fe55e7a213

📥 Commits

Reviewing files that changed from the base of the PR and between 42b3d3a and d49d0fb.

📒 Files selected for processing (5)
  • src/app/setup-api/hermes/models/route.ts
  • src/app/setup-api/hermes/provider-key/route.ts
  • src/components/AIModelsStep.tsx
  • src/components/HermesProviderConfig.tsx
  • src/lib/hermes-cli.ts

Comment thread src/app/setup-api/hermes/models/route.ts Outdated
Comment thread src/app/setup-api/hermes/provider-key/route.ts
Comment thread src/components/HermesProviderConfig.tsx Outdated
Comment thread src/lib/hermes-cli.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/lib/hermes-cli.ts (2)

33-37: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Sensitive Data Exposure (CWE-526)

Reachability: Internal

Allowlist the Hermes child environment.

...process.env forwards all server environment variables to Hermes. If sensitive values exist, Hermes or its extensions can access them. Pass only the variables required by Hermes.

🤖 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/hermes-cli.ts` around lines 33 - 37, Update the child-process
environment construction in the Hermes invocation to remove the broad
...process.env spread and explicitly allowlist only the variables Hermes
requires, retaining HOME and the constructed PATH values. Ensure no unrelated
server environment variables are forwarded.

30-31: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Sensitive Data Exposure (CWE-214)

Reachability: External

Keep provider API keys out of the child argument vector.

src/app/setup-api/hermes/provider-key/route.ts passes apiKey to spawn through --api-key. Same-host process inspection can expose the credential. Use a Hermes-supported stdin, file-descriptor, or secure-store flow. Ensure the replacement does not place the key in argv, the environment, or logs.

🤖 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/hermes-cli.ts` around lines 30 - 31, Update the Hermes process launch
around spawn in the hermes CLI flow so provider API keys are supplied through a
Hermes-supported secure stdin, file-descriptor, or secure-store mechanism
instead of --api-key. Ensure the key is absent from argv, environment variables,
and all logs, while preserving the existing child-process behavior for
non-secret arguments.
🤖 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/lib/hermes-cli.ts`:
- Around line 89-90: Update the opts.input handling around child.stdin.end in
the Hermes execution flow to attach an error listener before ending stdin,
routing stdin errors such as EPIPE through finish so the returned operation
rejects instead of producing an unhandled process error. Add a test covering
Hermes exiting before consuming input and asserting the operation rejects.

---

Outside diff comments:
In `@src/lib/hermes-cli.ts`:
- Around line 33-37: Update the child-process environment construction in the
Hermes invocation to remove the broad ...process.env spread and explicitly
allowlist only the variables Hermes requires, retaining HOME and the constructed
PATH values. Ensure no unrelated server environment variables are forwarded.
- Around line 30-31: Update the Hermes process launch around spawn in the hermes
CLI flow so provider API keys are supplied through a Hermes-supported secure
stdin, file-descriptor, or secure-store mechanism instead of --api-key. Ensure
the key is absent from argv, environment variables, and all logs, while
preserving the existing child-process behavior for non-secret arguments.
🪄 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: c18bffac-3102-4118-8eee-136a5705992f

📥 Commits

Reviewing files that changed from the base of the PR and between d49d0fb and 8cb04db.

📒 Files selected for processing (1)
  • src/lib/hermes-cli.ts

Comment thread src/lib/hermes-cli.ts Outdated
Comment on lines +89 to +90
if (opts.input !== undefined) {
child.stdin?.end(opts.input);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 'runHermesCli\(|input\s*:' src

node <<'NODE'
const { spawn } = require("node:child_process");

const child = spawn(
  process.execPath,
  ["-e", "process.exit(0)"],
  { stdio: ["pipe", "ignore", "ignore"] },
);

child.stdin.on("error", (error) => {
  console.log(`stdin error: ${error.code ?? error.message}`);
});
child.stdin.end("x".repeat(1024 * 1024));
child.on("close", (code, signal) => console.log({ code, signal }));
NODE

Repository: ID-Robots/clawbox

Length of output: 26495


🏁 Script executed:

#!/bin/bash
set -euo pipefail

cat -n src/lib/hermes-cli.ts | sed -n '1,150p'
printf '\n--- relevant callers ---\n'
rg -n -C 5 'runHermesCli\(' src/app src/lib src/tests

Repository: ID-Robots/clawbox

Length of output: 6667


🏁 Script executed:

#!/bin/bash
set +e

node <<'NODE'
const { spawn } = require("node:child_process");

const child = spawn(
  process.execPath,
  ["-e", "process.exit(0)"],
  { stdio: ["pipe", "ignore", "ignore"] },
);
child.stdin.end("x".repeat(1024 * 1024));
child.on("close", (code, signal) => console.log(JSON.stringify({ code, signal })));
NODE
status=$?
printf 'node_exit_status=%s\n' "$status"

Repository: ID-Robots/clawbox

Length of output: 671


Handle child.stdin errors explicitly.

When Hermes exits before consuming opts.input, child.stdin.end(opts.input) can emit an unhandled EPIPE error and terminate the Node.js process. Attach a child.stdin error listener and reject through finish. Add an early-exit test.

🧰 Tools
🪛 ast-grep (0.45.0)

[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 { spawn } from "child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 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/hermes-cli.ts` around lines 89 - 90, Update the opts.input handling
around child.stdin.end in the Hermes execution flow to attach an error listener
before ending stdin, routing stdin errors such as EPIPE through finish so the
returned operation rejects instead of producing an unhandled process error. Add
a test covering Hermes exiting before consuming input and asserting the
operation rejects.

Source: Path instructions

Separate Hermes skills store (distinct from the OpenClaw app store): search
the hermes skills registries, list installed (disk + hub lock), install/
uninstall via the hermes CLI. Hermes-themed, wired as a Hermes-edition-only
desktop app. All shell-outs go through runHermesCli (argv-only). Security:
URL installs disabled (registry identifiers only — no SSRF/untrusted-code
fetch); every route guarded to the Hermes active-harness; install verified
against the hub lock. Built by an orchestrated multi-agent pass + review.
ClawBox AI is a managed OpenAI-compatible provider, so it runs THROUGH Hermes
via a custom 'clawai' provider (base_url + device token) rather than a separate
custom path — only the device-login stays ClawBox-specific. New
/setup-api/hermes/clawai route configures it from the stored token+tier via
hermes config set (bare deepseek model id; Max=pro, Pro=flash); the panel shows
a ClawBox AI card with a one-click 'Use ClawBox AI'. Proven on-device (real
deepseek response through Hermes).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/app/page.tsx (2)

727-749: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Recompute mobileIconOrder when visibleDesktopApps changes.

visibleDesktopApps depends on harnessHiddenAppIds, which changes with activeHarness, but mobileIconOrder depends on desktopApps. React can reuse the previous harness’s mobile order after a harness toggle, causing newly visible mobile icons to fall back to { row: 0, col: 0 } and overlap other icons. Use visibleDesktopApps in the dependency list.

🤖 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/page.tsx` around lines 727 - 749, Update the mobileIconOrder useMemo
dependency list to include visibleDesktopApps instead of relying only on
desktopApps, ensuring the order is recomputed when harness visibility changes
and newly visible icons receive non-overlapping positions.

1038-1043: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Sensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information

Reachability: External

Force HTTPS on credential-bearing Hermes proxy hops.

This client opens the Hermes proxy at window.location.protocol, so LAN/desktop-only deployments served on http: open http://<hostname>:8090/. The proxy listens with Node http.createServer, forwards the valid clawbox_session SSO cookie to Hermes, and relays Hermes session cookies back. Use an HTTPS proxy for every credentialed hop, or otherwise exclude SSO cookie forwarding when the client origin is HTTP.

🤖 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/page.tsx` around lines 1038 - 1043, Update the Hermes URL
construction in the app launch flow to prevent credential-bearing proxy access
over HTTP: use an HTTPS proxy endpoint for hermes-dashboard, or conditionally
avoid forwarding SSO cookies when window.location.protocol is HTTP. Preserve the
existing hostname, proxy port, and non-Hermes app URL behavior.

Source: Path instructions

🤖 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/app/setup-api/hermes/skills/install/route.ts`:
- Around line 50-63: Remove the `isUrl` conditional and related comment in the
install route so `isInHubLock(skillName, id)` verification always runs before
reporting success. Remove the unused `isUrl` field from the `IdCheck` type and
all corresponding return values in `checkInstallIdentifier`.
- Around line 13-22: Update both POST handlers in
src/app/setup-api/hermes/skills/install/route.ts (lines 13-22) and
src/app/setup-api/hermes/skills/uninstall/route.ts (lines 11-20) to invoke a
shared CSRF/content-type validation helper under hermesSkillsGuard() before
parsing the body. Reject unsupported Content-Type values and validate the
request Origin or CSRF token, returning the helper’s rejection response before
request.json() or any Hermes CLI state change.

In `@src/app/setup-api/hermes/skills/search/route.ts`:
- Line 74: Implement shared server-side concurrency limiting inside runHermesCli
in src/lib/hermes-cli.ts so all Hermes CLI callers inherit a single-flight or
bounded-concurrency slot and immediately return a 429 response when unavailable.
Apply this behavior to src/app/setup-api/hermes/skills/search/route.ts lines
74-74, src/app/setup-api/hermes/skills/install/route.ts lines 46-46, and
src/app/setup-api/hermes/skills/uninstall/route.ts lines 28-28; each site must
acquire the same limiter before its CLI call and return 429 when the limit is
reached, while preserving the existing timeouts.

In `@src/components/HermesSkillsStore.tsx`:
- Around line 522-527: Update the result card rendered in results.map to be
keyboard-accessible by replacing the clickable div with a button, or by adding
an appropriate button role, tabIndex, and keyboard activation handler while
preserving setSelected(skill). Keep the existing nested Install and Remove
button stopPropagation behavior intact.
- Around line 154-163: Update setProgressAutoClear to store each setTimeout
handle in a ref, remove handles after they fire, and add unmount cleanup that
clears all tracked timers. Ensure the cleanup is wired through the component’s
existing lifecycle hooks without changing progress behavior.
- Around line 112-125: Fix both react-hooks/set-state-in-effect violations in
the effects near fetchInstalled and the debounced search logic: derive
empty-query display state during render instead of calling setResults,
setSearchError, or setLoading from the search effect, then use visibleResults
and visibleError at the indicated render and error-handling call sites to
prevent stale results. Keep the effect focused on fetching non-empty browse
queries, and address the fetchInstalled mount effect by either moving the
initial load to an event-driven path or adding a narrowly scoped suppression
with a clear reason.
- Around line 296-310: Replace the confirm-install overlay `<div>` with a native
`<dialog>` and manage its lifecycle with `showModal()` when `confirmInstall` is
set and `close()` when it clears, preserving the existing body contents and
`aria-labelledby="hs-confirm-title"`. Keep the dialog mounted if required by the
existing effect, guard the title with `confirmInstall?.name`, and ensure closing
restores focus to the install trigger while native dialog behavior handles
Escape, focus trapping, and the backdrop.

In `@src/lib/hermes-skills-server.ts`:
- Around line 145-176: Update enumerateInstalledSkills to bound directory
traversal and discovered skills with explicit caps, stopping walkForSkillMd
processing once either limit is reached while preserving the hub-lock overlay.
Add short-TTL caching for the assembled installed-skill list so repeated calls
avoid rescanning, and invalidate that cache in the install and uninstall flows
after successful changes.
- Around line 52-62: Update readHubLock to return an empty lock only when
fs.readFile fails with ENOENT; propagate permission, directory, and JSON parsing
errors instead. Ensure callers such as enumerateInstalledSkills and the install
route can catch the propagated error and report the actual hub-lock failure
rather than treating it as an unresolved skill.

---

Outside diff comments:
In `@src/app/page.tsx`:
- Around line 727-749: Update the mobileIconOrder useMemo dependency list to
include visibleDesktopApps instead of relying only on desktopApps, ensuring the
order is recomputed when harness visibility changes and newly visible icons
receive non-overlapping positions.
- Around line 1038-1043: Update the Hermes URL construction in the app launch
flow to prevent credential-bearing proxy access over HTTP: use an HTTPS proxy
endpoint for hermes-dashboard, or conditionally avoid forwarding SSO cookies
when window.location.protocol is HTTP. Preserve the existing hostname, proxy
port, and non-Hermes app URL behavior.
🪄 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: 775f8c3c-e2f6-48fa-8aa2-997d4b1ce10d

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb04db and 597a09d.

📒 Files selected for processing (8)
  • src/app/page.tsx
  • src/app/setup-api/hermes/skills/install/route.ts
  • src/app/setup-api/hermes/skills/installed/route.ts
  • src/app/setup-api/hermes/skills/search/route.ts
  • src/app/setup-api/hermes/skills/uninstall/route.ts
  • src/components/HermesSkillsStore.tsx
  • src/lib/hermes-skills-server.ts
  • src/lib/hermes-skills.ts

Comment thread src/app/setup-api/hermes/skills/install/route.ts
Comment thread src/app/setup-api/hermes/skills/install/route.ts
args.push("--source", source);
}

const r = await runHermesCli(args, { timeoutMs: 60_000 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

No route limits how many Hermes CLI processes run at once on the embedded target. Each handler spawns a hermes process per request with a long timeout, and runHermesCli in src/lib/hermes-cli.ts applies no concurrency control. The first search builds a large index, as the comment at Lines 13-16 of the search route states. Repeated or concurrent requests therefore multiply CPU and memory use on the Jetson device, and the client-side 300 ms debounce does not bound server-side work. The shared fix is one server-side concurrency limit around runHermesCli.

  • src/app/setup-api/hermes/skills/search/route.ts#L74-L74: acquire a shared single-flight or bounded-concurrency slot before the 60 s call, and return 429 when the limit is reached.
  • src/app/setup-api/hermes/skills/install/route.ts#L46-L46: acquire the same slot before the 120 s call, and return 429 when the limit is reached.
  • src/app/setup-api/hermes/skills/uninstall/route.ts#L28-L28: acquire the same slot before the 30 s call, and return 429 when the limit is reached.

Implement the limiter inside src/lib/hermes-cli.ts so every current and future caller inherits it.

📍 Affects 3 files
  • src/app/setup-api/hermes/skills/search/route.ts#L74-L74 (this comment)
  • src/app/setup-api/hermes/skills/install/route.ts#L46-L46
  • src/app/setup-api/hermes/skills/uninstall/route.ts#L28-L28
🤖 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/hermes/skills/search/route.ts` at line 74, Implement shared
server-side concurrency limiting inside runHermesCli in src/lib/hermes-cli.ts so
all Hermes CLI callers inherit a single-flight or bounded-concurrency slot and
immediately return a 429 response when unavailable. Apply this behavior to
src/app/setup-api/hermes/skills/search/route.ts lines 74-74,
src/app/setup-api/hermes/skills/install/route.ts lines 46-46, and
src/app/setup-api/hermes/skills/uninstall/route.ts lines 28-28; each site must
acquire the same limiter before its CLI call and return 429 when the limit is
reached, while preserving the existing timeouts.

Source: Path instructions

Comment thread src/components/HermesSkillsStore.tsx Outdated
Comment thread src/components/HermesSkillsStore.tsx
Comment thread src/components/HermesSkillsStore.tsx Outdated
Comment on lines +296 to +310
const confirmModal = confirmInstall && (
<div
className="fixed inset-0 z-[9999] flex items-center justify-center bg-black/60 backdrop-blur-sm"
onClick={() => setConfirmInstall(null)}
onKeyDown={(e) => {
if (e.key === "Escape") setConfirmInstall(null);
}}
role="dialog"
aria-modal="true"
aria-labelledby="hs-confirm-title"
>
<div
className="card-surface rounded-2xl p-6 max-w-sm mx-4 shadow-2xl"
onClick={(e) => e.stopPropagation()}
>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Fix keyboard access in the install-confirm modal.

The modal has three defects for keyboard users:

  • onKeyDown sits on a div that cannot receive focus, so Escape never reaches the handler.
  • No element receives focus when the modal opens, so the Install and Cancel buttons are not reachable without tabbing through the page behind the backdrop.
  • Focus is not trapped and not restored to the trigger on close.

The modal is the only path to confirm an install, so a keyboard user cannot complete the task.

Use the native <dialog> element with showModal(). It provides focus trapping, Escape to close, and the backdrop.

♿ Proposed fix using the native dialog element
-import { useCallback, useEffect, useMemo, useState } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
+  const confirmDialogRef = useRef<HTMLDialogElement | null>(null);
+  useEffect(() => {
+    const el = confirmDialogRef.current;
+    if (!el) return;
+    if (confirmInstall && !el.open) el.showModal();
+    if (!confirmInstall && el.open) el.close();
+  }, [confirmInstall]);
+
   // ── Install-confirm modal ──
-  const confirmModal = confirmInstall && (
-    <div
-      className="fixed inset-0 z-[9999] flex items-center justify-center bg-black/60 backdrop-blur-sm"
-      onClick={() => setConfirmInstall(null)}
-      onKeyDown={(e) => {
-        if (e.key === "Escape") setConfirmInstall(null);
-      }}
-      role="dialog"
-      aria-modal="true"
-      aria-labelledby="hs-confirm-title"
-    >
-      <div
-        className="card-surface rounded-2xl p-6 max-w-sm mx-4 shadow-2xl"
-        onClick={(e) => e.stopPropagation()}
-      >
+  const confirmModal = (
+    <dialog
+      ref={confirmDialogRef}
+      onClose={() => setConfirmInstall(null)}
+      aria-labelledby="hs-confirm-title"
+      className="z-[9999] backdrop:bg-black/60 backdrop:backdrop-blur-sm bg-transparent p-0 border-0"
+    >
+      <div className="card-surface rounded-2xl p-6 max-w-sm mx-4 shadow-2xl">

Close the element at the end of the block, and keep the body contents unchanged. Render {confirmInstall && <dialog …>} only if you prefer to unmount it; the useEffect above assumes it stays mounted. Guard the title text with confirmInstall?.name.

🧰 Tools
🪛 React Doctor (0.9.3)

[warning] 303-303: Keyboard users can tab out of this role="dialog" modal because it has no built-in focus trapping, so use the native <dialog>, which gives you focus trapping, Escape to close, and the backdrop for free.

Replace the wrapper with <dialog> and open it with dialog.showModal(). For the trigger, prefer <button commandfor="id" command="show-modal"> (Chrome 135+), or a useRef with dialogRef.current?.showModal().

(prefer-html-dialog)

🤖 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/HermesSkillsStore.tsx` around lines 296 - 310, Replace the
confirm-install overlay `<div>` with a native `<dialog>` and manage its
lifecycle with `showModal()` when `confirmInstall` is set and `close()` when it
clears, preserving the existing body contents and
`aria-labelledby="hs-confirm-title"`. Keep the dialog mounted if required by the
existing effect, guard the title with `confirmInstall?.name`, and ensure closing
restores focus to the install trigger while native dialog behavior handles
Escape, focus trapping, and the backdrop.

Sources: Path instructions, Linters/SAST tools

Comment thread src/components/HermesSkillsStore.tsx Outdated
Comment thread src/lib/hermes-skills-server.ts
Comment thread src/lib/hermes-skills-server.ts
…isting

Provider panel now uses the OpenClaw AI-provider radio-card UI (logos via
AIProviderIcon) instead of plain dropdowns; ClawBox AI is the top card. Added
custom provider marks for the Hermes-only providers (nous, z.ai, kimi, copilot,
auto) + icon aliases (gemini->google, codex->openai). Skills store Browse now
shows a default listing via a new /setup-api/hermes/skills/browse route that
parses 'hermes skills browse' at a wide COLUMNS (registry-only, edition-gated);
typing still searches. runHermesCli gains an env option (for COLUMNS).
…n panel

Surface Hermes' own provider OAuth in the AI-provider panel. New
/setup-api/hermes/oauth reads the dashboard's /api/providers/oauth status via a
server-side SSO login helper (hermes-dashboard-auth), so the panel shows which
OAuth providers are connected. OAuth-capable providers (anthropic PKCE,
openai-codex device-code, nous, copilot) get a Sign in / Connected affordance
that launches Hermes' native /env OAuth flow through the auth-gated proxy —
no reimplementation of PKCE/device-code, and API-key stays available where
the provider supports both.
Comment thread src/lib/hermes-dashboard-auth.ts Fixed
…e UI

New /setup-api/hermes/skills/inspect route: parses 'hermes skills inspect'
panels (COLUMNS=200) for name/description/source/trust/tags/version/author/
license, and reads the FULL untruncated SKILL.md off disk for installed skills
(hub lock install_path, traversal-guarded) instead of the truncated CLI
preview. Derives a source link only when confident (github/browse-sh hosts,
skills.sh Repo/Detail Page) and surfaces setup links (setup.help,
collect_secrets provider_url). Store gets a real detail view (rendered
SKILL.md via the existing chat-markdown renderer), richer cards with trust/
source/category/scan chips, skeletons, and better empty/error states.
PROVIDER STACK
- Models are now provider-SCOPED from Hermes' own /api/model/options (live,
  per-provider) instead of filtering vendor prefixes off a shared catalog —
  the prefix approach silently produced invalid ids (OpenRouter spells it
  anthropic/claude-opus-4.8, direct Anthropic spells it claude-opus-4-8).
  A scoped GET blanks `current` when the saved model belongs to another
  provider, so a foreign id can't reach the browser; the POST rejects a
  mismatched pairing server-side (shouldEnforcePairing distinguishes an
  unauthenticated provider, which serves nothing, from a credentialed one we
  merely couldn't enumerate).
- The ClawBox AI card is now literally the same components in both panels
  (ClawboxAiProviderRow/PlanPicker/DeviceLogin), so they cannot drift.
- No OpenClaw flash on a Hermes device: the panel renders a neutral skeleton
  until the edition resolves.
- Chat popup gained provider + model + thinking-effort, routed to
  `hermes -z --provider --reasoning` with argv-only validation.
- The hardcoded model fallback list is gone; the catalogue is live with SWR.

SKILLS APP
- Browse is served from Hermes' own 90k-skill offline index (paging, search,
  facets, no CLI, works offline). `inspect` is only used where it is lossless:
  Rich strips unquoted YAML flow sequences, so platforms/tags/related_skills
  are read from disk, never from CLI output.
- Far richer detail (provenance, security scan, requirements, size) and a
  rebuilt UI. The OpenClaw app store is hidden on the Hermes edition.

AUDIT HARDENING (from a five-auditor review of the shipped work)
- install.sh completes on every edition again: the service drift guard now
  knows units that are deliberately absent, so fresh installs AND the in-app
  updater stop failing on both SKUs.
- The Hermes SKU actually removes the OpenClaw gateway (stop, disable, remove
  unit, mask) instead of merely skipping its install — it was left running,
  unauthenticated, on 0.0.0.0:18789.
- The edition is persisted to root-owned /etc/clawbox/edition.env, read by the
  web server and the updater, so updates stop reinstalling OpenClaw onto a
  Hermes box. A missing licence key now means LOCKED, not unlocked.
- Factory reset clears the Hermes credential store (surgically — the 1.2 GB
  agent install stays), and the dashboard password script verifies hash
  against plaintext so a reset can no longer desync it forever.
- Pre-setup middleware gates hermes/chat, hermes/skills and harness/select
  (open-AP window) while leaving the wizard's provider routes reachable.
- websockify binds 127.0.0.1: it was republishing a -nopw x11vnc desktop to
  the whole LAN, bypassing the authenticated /novnc-ws proxy.
- Dashboard proxy: Host/Origin guard at the edge, absolute login redirect
  (was an infinite loop), working 401 re-login, no pipe-after-end hang.
Switching provider made the chat header collapse to just Provider + Thinking
for a second, then reflow when the model list arrived — two layout shifts for
one click.

The cause is deliberate and should stay: useHermesModelOptions drops its scope
the moment the provider changes, because continuing to show the old provider's
models is exactly the foreign-vendor mismatch it exists to prevent. What was
wrong was the consequence — the pill unmounted instead of holding its place.

It now stays mounted through the load, disabled, showing an ellipsis rather
than the previous provider's model id: blank for a beat is better than wrong
for a beat. A provider that genuinely serves one model still gets no picker,
because the remembered state only updates once loading settles.
Decode entities in a single pass instead of a chain of replaces, so an
already-escaped sequence is not decoded twice. Match script/style end tags
that carry trailing whitespace, drop an unterminated script/style block, and
strip tags to a fixed point so one pass cannot leave a tag behind.

Share the strip helper with the search-result parser.
Comment thread mcp/lib/web.ts Fixed
Comment thread mcp/lib/web.ts Fixed
Comment thread mcp/lib/web.ts Fixed
The Hermes edition is a new device SKU rather than a point fix on the 3.1
line, so it takes the minor bump.
An end tag may legally carry attribute-like text before its ">", so anchor
the pattern on the first ">" after the tag name rather than on whitespace,
and require whitespace or ">" after the name so a longer name is not matched.

Apply the removal to a fixed point and fold the three raw-text elements into
one helper.
The specs were rewritten for the wizard's new disclosures but never
executed. Running them turned up three failures, only one of which was
about the disclosures.

The provider list settles in two beats: the edition resolves, and then
the provider in play lands. Only the second one collapses the list, so a
point-in-time count of the "show more providers" toggle taken between
them reads a list that has nothing to open yet, skips the expansion, and
the list closes over the rows the test is about to assert. Waiting for
the radiogroup covered the first beat only. The checked radio is the
second beat's signal — the list cannot collapse before one exists — so
the count that follows reads a list that has stopped moving. The click
now also waits for the toggle to go away, which is what "fully open"
means. e2e-install carries the same wait; a real device settles slower
than anything else, so it needs it most.

The chat app's label became the translated "Chat", where it used to be
the unique "Claw". Three controls now answer to that name — the desktop
icon, the shelf's app icon, and the shelf toggle — and only the toggle
opens the popup; the other two open a window. The four chat-popup specs
asked for it by name and got an ambiguous match. They now go through the
shelf toggle, which gains the data-testid every other button in that
shelf already had.

The mock owns every /setup-api path and ends in a catch-all `{}`, which
answers /setup-api/harness/active with neither `active` nor `edition`.
The desktop reads an unknown harness as "hide both harnesses' apps" and
fails closed, which quietly took the App Store and the OpenClaw Control
UI off the shelf — the two apps store-flow and installed-app-settings
drive. The mocked device now says what it is.

Full suite on linux/chromium, CI settings: 41 passed, 2 pre-existing
fixmes skipped, e2e bundle coverage 47.9% against a floor of 39.
…fits

The checks around LLAMACPP_PREBUILT were fitness checks — right architecture,
right backend, starts on this device — and the comment above them claimed more
than that. Nothing established the archive was the one we meant to ship, and
the --version probe runs it as root.

Plain http is now refused rather than downgraded to: the intended use is a
factory bench fetching from a build host, where trusting the network would make
a MITM a persistent root implant on every device flashed. And an optional
LLAMACPP_PREBUILT_SHA256 is verified BEFORE the archive is unpacked or run,
because a digest checked afterwards proves nothing about what already executed.

Optional rather than required: the common case is a file copied over SSH by the
same operator running the install, where the channel is already the guarantee.
The minting tool accepted any --days value and passed it through Number(),
so a value that is not a number produced a licence whose expiry the device
could not read while the operator believed a term had been set. Refuse the
argument up front, and make the verifier treat a present-but-unreadable exp
as invalid rather than as absent. The field checks move into an exported
isLicensePayloadValid() so they can be exercised on their own.
A child that exits before draining its input makes the write fail on the
pipe, and a stream error with no listener does not reach the promise. Route
it through finish(), deferred one turn so a child that finished its work
without reading the input still reports its own result.
ui_language is the one preference interpolated into the agent's persona
files. The POST route validates it, but that is a property of the call
graph; move the writer into src/lib/language-persona.ts and repeat the
closed-domain check there so it holds for the function on its own.
On a single-harness edition the badge is the only health signal the user
gets, and its dot was hardcoded green. Use the same dot convention as the
switcher below it.
HERMES_DASH_USERNAME is operator-set and was written as a bare YAML scalar
while the two fields beside it were quoted. A value holding ':' or '#', or a
leading '-', changes what the line means or ends the document — and the
dashboard refuses to start without an auth provider to load.
…fuses

The bare rejects.toThrow() matchers accepted any throw, including a
misconfigured mock. Pin the message, and assert the property the lock
actually exists to protect: a refused switch writes nothing.
The status route body is unvalidated JSON and the list can be missing. The
active-entry lookup added with the locked badge ran on every render, so an
absent list threw during the render triggered by the fetch. The picker sits in
Settings -> System with no boundary above it, so the throw unmounted the whole
desktop tree and blanked every panel.

Type the field as optional (it already had an `?? []` fallback in the switcher
branch) and reach for it with the same optional chain.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci-e2e Auto-triage area area: gateway Auto-triage area area: install Auto-triage area area: ui Auto-triage area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants