Skip to content

fix: hermes edition handling for status, configure, catalog and clawkeep - #364

Merged
KrasimirKralev merged 4 commits into
ID-Robots:betafrom
KrasimirKralev:fix/hermes-edition-leaks
Aug 11, 2026
Merged

KrasimirKralev merged 4 commits into
ID-Robots:betafrom
KrasimirKralev:fix/hermes-edition-leaks

Conversation

@KrasimirKralev

@KrasimirKralev KrasimirKralev commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Server-side fixes for OpenClaw assumptions that survive onto a Hermes device.

Found by flashing a device and using it, not in CI β€” the unit suite cannot see a
tool that isn't installed.

What this fixes

  • Remote Control kept asking a signed-in device to sign in. The AI status
    route read its answer from the wrong place on this edition.

  • Operations that need the OpenClaw CLI ran on the edition that doesn't ship
    it
    , failing late and opaquely inside a request. There is now a typed
    edition check at the shared entry point, and the callers that reached it β€”
    model-catalog refresh, local-AI disable, the updater probes β€” skip cleanly.
    openclaw and dual editions are unaffected.

  • The local model (Gemma) switch and API-key providers failed to configure.
    The configure route took the OpenClaw path for every provider. On this edition
    it now uses the Hermes equivalents, and a failure no longer reports a
    credential problem for a local model that has no credentials.

  • ClawKeep offered a remedy that contradicts the edition. It depends on the
    OpenClaw agent, so it cannot run here; it now says so plainly instead of
    printing an install command. Copy added for all ten locales.

Tests

A regression test per fix, covering the edition-conditional behaviour in each β€”
status resolution, the guarded entry point, catalog refresh, the three configure
paths, error classification, and ClawKeep availability.

Verification

Reproduced read-only on a flashed device before fixing. Post-fix on-device
verification and the Linux build/e2e run are still outstanding β€” the build host
was unreachable while this was written.

…ermes edition

The status route read only `~/.openclaw/openclaw.json`, which does not exist on a Hermes device, so a signed-in box reported clawaiConfigured=false and the Remote Control panel kept prompting the user to sign in. Resolve the active provider, model and ClawBox AI profile from the Hermes config (via the mtime-memoised CLI helper) when the active harness is Hermes, keeping the OpenClaw path unchanged.
The Hermes SKU installs no openclaw binary, so findOpenclawBin's bare "openclaw" fallback turned every spawn into a raw "spawn openclaw ENOENT" deep inside a request. Add a typed OpenclawUnavailableError and an openclawIsAbsent() edition check, guard the shared spawnOpenclaw chokepoint with it, and skip the openclaw spawns in the model-catalog refresh, the local-ai disable path and the updater version/doctor probes. openclaw and dual editions are unaffected.
The configure route wrote OpenClaw config and shelled out to `openclaw config set` for every provider, so on Hermes the local-model (Gemma) switch and API-key providers failed with a spawn ENOENT and then told the user to check credentials that a local model does not have. On the hermes edition, route local models to applyLocalAiToHermes, ClawBox AI to applyClawaiToHermes, and API-key cloud providers to the Hermes credential store, and classify the catch-all error so a local or edition failure never reports a credential problem.
ClawKeep archives the OpenClaw agent through the openclaw CLI, so on the Hermes SKU it cannot run β€” yet it advised installing OpenClaw (`npm install -g openclaw`), which contradicts the edition. Surface supportedOnEdition from getStatus and, when false, show an honest "not available on this edition" card instead of the install instructions, with new copy in all ten locales.
@KrasimirKralev
KrasimirKralev requested a review from a team as a code owner August 11, 2026 08:23
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

πŸ“ Walkthrough

Walkthrough

The PR adds Hermes-aware AI model configuration and status handling, prevents OpenClaw CLI calls on unsupported editions, updates ClawKeep edition handling, and adds related tests and translations.

Changes

Hermes edition support

Layer / File(s) Summary
Edition-aware OpenClaw guards
src/lib/openclaw-config.ts, src/lib/updater.ts, src/app/setup-api/ai-models/catalog/route.ts, src/app/setup-api/local-ai/route.ts, src/tests/**
Hermes editions now skip unavailable OpenClaw operations. Typed errors identify unsupported CLI use.
Hermes AI model configuration
src/app/setup-api/ai-models/configure/route.ts, src/lib/hermes-cloud-provider.ts, src/tests/routes/ai-models/*
Hermes configures local models, ClawBox AI, and supported API-key providers. Unsupported OAuth and subscription flows return explicit errors.
Harness-aware AI status
src/app/setup-api/ai-models/status/route.ts, src/tests/routes/ai-models/status.test.ts
Status resolution now supports OpenClaw and Hermes state, including ClawBox AI account data and portal tiers.
ClawKeep edition handling
src/lib/clawkeep.ts, src/components/ClawKeepApp.tsx, src/lib/clawkeep-translations.ts, src/tests/unit/clawkeep-edition.test.ts
ClawKeep reports edition support and displays localized unavailable-edition content on Hermes.

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

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ConfigureRoute
  participant HermesCloudProvider
  participant HermesCLI
  Client->>ConfigureRoute: submit provider configuration
  ConfigureRoute->>HermesCloudProvider: apply API key
  HermesCloudProvider->>HermesCLI: store credentials and resolve model
  HermesCLI-->>HermesCloudProvider: provider and model result
  HermesCloudProvider-->>ConfigureRoute: activation result
  ConfigureRoute-->>Client: configuration response
Loading

Possibly related PRs

Suggested labels: bug, area: gateway

Suggested reviewers: yalexx

πŸš₯ Pre-merge checks | βœ… 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
βœ… Passed checks (4 passed)
Check name Status Explanation
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.
Title check βœ… Passed The title clearly summarizes the main Hermes-edition fixes across status, configuration, catalog handling, and ClawKeep.
Description check βœ… Passed The description clearly explains the changes, affected behaviors, regression tests, and outstanding verification work.
✨ Finishing Touches
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

πŸ¦€ ClawReview

Claws waving β€” here's what this change is about.

This PR patches four places where code written for OpenClaw devices accidentally ran on Hermes β€” where the openclaw binary and ~/.openclaw config don't exist. The fixes cover: the AI-models status route (now reads Hermes config when the active harness is Hermes), a new typed guard at the spawnOpenclaw chokepoint, the configure route (routes local/ClawBox-AI/API-key providers through Hermes' own config store instead of shelling out), and ClawKeep (surfaces an honest 'not available on this edition' card instead of the misleading npm install remedy). Each fix ships with a regression test.

At a glance

  • πŸ”§ Fix Β· touches AI-models status/configure/catalog routes, ClawKeep app, openclaw-config lib, updater version probe, and a new hermes-cloud-provider helper
  • Base branch: beta Β· +539 source / +444 tests across 17 files
  • βœ… base beta matches the beta-first convention
  • βœ… conventional PR title
  • βœ… source changes come with test changes
  • 🟑 large PR (1107 lines changed) β€” consider splitting

Good to know

  • ℹ️ Adds a new source file src/lib/hermes-cloud-provider.ts β€” a new module in the runtime bundle that routes API-key cloud providers to Hermes' credential store.
  • 🟑 Touches the updater's version-probe and doctor-fix paths, both of which run on customer devices during the auto-update flow.
  • ℹ️ ClawKeep translation strings added for all 10 locales β€” worth a quick spot-check that the copy reads naturally in context.
  • ℹ️ Policy flagged this as a large PR (1,107 lines across 17 files); the four fixes are logically independent and could have been split, but each ships with tests.

β€” ClawReview πŸ¦€. I set the scene; CodeRabbit reviews the code; you decide. Conventions: docs.

@github-actions github-actions Bot added area: gateway Auto-triage area area: ui Auto-triage area labels Aug 11, 2026
@KrasimirKralev KrasimirKralev changed the title fix: server-side Hermes edition leaks (status, openclaw spawn sweep, configure, ClawKeep) fix: hermes edition handling for status, configure, catalog and clawkeep Aug 11, 2026

@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: 10

πŸ€– 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/ai-models/configure/route.ts`:
- Around line 702-722: Update the Branch A condition in the openclawIsAbsent()
flow to recognize local providers by provider identity, not only isLocalScope:
route llama.cpp and Ollama requests through the existing local configuration and
applyLocalAiToHermes logic even when scope is "primary". Preserve the current
shouldPromoteLocalToPrimary behavior and model normalization, while leaving
non-local providers on their existing branches.
- Around line 709-720: Reorder the local-model setup around applyLocalAiToHermes
so Hermes activation succeeds before persisting local_ai_configured,
local_ai_provider, local_ai_model, and local_ai_configured_at via setMany. Keep
the existing provider and model normalization, then perform the state write only
after successful activation, matching the ordering used by
applyCloudProviderKeyToHermes.
- Around line 735-741: Update the Hermes setup flow around result.activated so
stored but unactivated keys return a 2xx success response, or explicitly handle
the 409 continuation path in AIModelsStep.tsx. Add an error discriminator to the
Hermes error classes and map validation failures, including missing or malformed
local model IDs and missing API keys, to HTTP 400 while preserving HTTP 502 for
Hermes CLI failures.

In `@src/app/setup-api/ai-models/status/route.ts`:
- Around line 314-315: Before the portal-token request that uses clawaiToken,
validate CLAWBOX_AI_DEVICE_INFO_URL as a trusted HTTPS origin using an explicit
allowlist, and reject non-HTTPS or unapproved origins before sending the Bearer
token. Configure the request to use redirect: "error" so the token cannot follow
an unvalidated redirect.

In `@src/lib/hermes-cloud-provider.ts`:
- Around line 92-99: Update the catch block in the model-option loading flow
around getModelOptions, isAllowedProvider, and scopeFromPayload to emit a
warning containing the caught error and relevant provider context before
preserving the existing fallback behavior. Use the module’s established logging
mechanism and keep returning with no selected model after logging.
- Around line 80-87: Replace raw Hermes stderr with authored, secret-safe error
messages in the failure paths around runHermesCli in
src/lib/hermes-cloud-provider.ts (lines 80-87 and 112-119),
src/lib/hermes-local-ai.ts, and src/lib/hermes-clawai.ts; log only redacted
diagnostics server-side. Update the configure route comment in
src/app/setup-api/ai-models/configure/route.ts (lines 749-757) to state that
client responses use authored messages rather than helper stderr.

In `@src/tests/routes/ai-models/catalog-edition.test.ts`:
- Line 9: Update the child_process mock’s spawn stub to return a
child-process-shaped object with the required stdout stream and event-listener
methods, and asynchronously emit close so the pending timeout is cleared.
Preserve the existing mock behavior while ensuring synchronous child.stdout
access succeeds and the Promise settles through the close path.

In `@src/tests/routes/ai-models/configure-hermes.test.ts`:
- Around line 155-169: Add a test in the local-model suite that posts a
llama.cpp configuration without the scope field, using the default primary scope
request shape, and assert the response succeeds and mockApplyLocalAiToHermes is
called. Keep the existing explicit scope: "local" coverage unchanged and retain
the no-OpenClaw assertion for this default-scope case.

In `@src/tests/routes/ai-models/configure.test.ts`:
- Around line 71-75: Re-seed openclawIsAbsent after vi.clearAllMocks() in both
test suites: import it in src/tests/routes/ai-models/configure.test.ts and
src/tests/routes/local-ai.test.ts, then add
vi.mocked(openclawIsAbsent).mockReturnValue(false) to each suite’s beforeEach
re-seed block. Apply the corresponding changes at
src/tests/routes/ai-models/configure.test.ts lines 71-75 and
src/tests/routes/local-ai.test.ts lines 17-19; both sites require the same fix.

In `@src/tests/unit/openclaw-config.test.ts`:
- Around line 123-128: Add spawn: vi.fn() to the child_process mock used by the
openclawConfig tests, then update the Hermes runOpenclawConfigSet test to assert
spawn was not called after the OpenclawUnavailableError rejection. Keep the
existing execFile mock and typed-error assertion unchanged.
πŸͺ„ 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: 02979dd1-84bf-47d0-9ef2-c12789de88cd

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between a32435c and 8c82c72.

πŸ“’ Files selected for processing (17)
  • src/app/setup-api/ai-models/catalog/route.ts
  • src/app/setup-api/ai-models/configure/route.ts
  • src/app/setup-api/ai-models/status/route.ts
  • src/app/setup-api/local-ai/route.ts
  • src/components/ClawKeepApp.tsx
  • src/lib/clawkeep-translations.ts
  • src/lib/clawkeep.ts
  • src/lib/hermes-cloud-provider.ts
  • src/lib/openclaw-config.ts
  • src/lib/updater.ts
  • src/tests/routes/ai-models/catalog-edition.test.ts
  • src/tests/routes/ai-models/configure-hermes.test.ts
  • src/tests/routes/ai-models/configure.test.ts
  • src/tests/routes/ai-models/status.test.ts
  • src/tests/routes/local-ai.test.ts
  • src/tests/unit/clawkeep-edition.test.ts
  • src/tests/unit/openclaw-config.test.ts

Comment on lines +702 to +722
if (openclawIsAbsent()) {
try {
if (isLocalScope && (isLlamaCpp || isOllama)) {
// On-device model (Gemma via llama.cpp, or Ollama): persist the same
// config-store keys the OpenClaw path would, then register it with
// Hermes as an available provider (activating it only on a fresh
// device, matching shouldPromoteLocalToPrimary). No gateway restart.
await setMany({
local_ai_configured: true,
local_ai_provider: ocProvider,
local_ai_model: config.defaultModel,
local_ai_configured_at: new Date().toISOString(),
});
await applyLocalAiToHermes({
provider: ocProvider as "llamacpp" | "ollama",
// Hermes wants the bare model id, not the `llamacpp/…` qualified form.
model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
makeDefault: shouldPromoteLocalToPrimary,
});
return NextResponse.json({ success: true });
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | πŸ—οΈ Heavy lift

Local providers with primary scope fall into the cloud-provider branch on Hermes.

Branch A gates on isLocalScope. A llama.cpp or Ollama request with the default scope: "primary" skips it.

Trace { provider: "llamacpp", apiKey: "gemma-q4" } (primary scope β€” the shape configure.test.ts Line 665 already posts):

  1. Line 704: isLocalScope is false, so Branch A does not run.
  2. Line 723: not ClawAI.
  3. Line 727: authMode is "token" and normalizedApiKey is "gemma-q4", so Branch C runs.
  4. applyCloudProviderKeyToHermes({ openclawProvider: "llamacpp", … }) calls hermesKeyProviderFor("llamacpp"), which returns null.
  5. The helper throws HermesCloudApplyError, and the route returns 502 "This provider is set up through the Hermes provider panel on this edition."

For llama.cpp the apiKey field carries the model name, not a credential β€” Line 627 documents this. The on-device model is therefore treated as an unmapped cloud provider.

{ provider: "llamacpp" } with no apiKey is also wrong: normalizedApiKey is empty, so Branch C is skipped and the Line 745 fallback returns 400 with the same provider-panel message.

Gate Branch A on the provider, not the scope.

πŸ› Proposed fix for the branch gate
-        if (isLocalScope && (isLlamaCpp || isOllama)) {
+        if (isLlamaCpp || isOllama) {
           // On-device model (Gemma via llama.cpp, or Ollama): persist the same
           // config-store keys the OpenClaw path would, then register it with
           // Hermes as an available provider (activating it only on a fresh
           // device, matching shouldPromoteLocalToPrimary). No gateway restart.
           await setMany({
             local_ai_configured: true,
             local_ai_provider: ocProvider,
             local_ai_model: config.defaultModel,
             local_ai_configured_at: new Date().toISOString(),
           });
           await applyLocalAiToHermes({
             provider: ocProvider as "llamacpp" | "ollama",
             // Hermes wants the bare model id, not the `llamacpp/…` qualified form.
             model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
-            makeDefault: shouldPromoteLocalToPrimary,
+            // A primary-scope request selects the model explicitly, so activate
+            // it; a local-scope request activates only on a fresh device.
+            makeDefault: !isLocalScope || shouldPromoteLocalToPrimary,
           });
           return NextResponse.json({ success: true });
         }
πŸ“ 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
if (openclawIsAbsent()) {
try {
if (isLocalScope && (isLlamaCpp || isOllama)) {
// On-device model (Gemma via llama.cpp, or Ollama): persist the same
// config-store keys the OpenClaw path would, then register it with
// Hermes as an available provider (activating it only on a fresh
// device, matching shouldPromoteLocalToPrimary). No gateway restart.
await setMany({
local_ai_configured: true,
local_ai_provider: ocProvider,
local_ai_model: config.defaultModel,
local_ai_configured_at: new Date().toISOString(),
});
await applyLocalAiToHermes({
provider: ocProvider as "llamacpp" | "ollama",
// Hermes wants the bare model id, not the `llamacpp/…` qualified form.
model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
makeDefault: shouldPromoteLocalToPrimary,
});
return NextResponse.json({ success: true });
}
if (openclawIsAbsent()) {
try {
if (isLlamaCpp || isOllama) {
// On-device model (Gemma via llama.cpp, or Ollama): persist the same
// config-store keys the OpenClaw path would, then register it with
// Hermes as an available provider (activating it only on a fresh
// device, matching shouldPromoteLocalToPrimary). No gateway restart.
await setMany({
local_ai_configured: true,
local_ai_provider: ocProvider,
local_ai_model: config.defaultModel,
local_ai_configured_at: new Date().toISOString(),
});
await applyLocalAiToHermes({
provider: ocProvider as "llamacpp" | "ollama",
// Hermes wants the bare model id, not the `llamacpp/…` qualified form.
model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
// A primary-scope request selects the model explicitly, so activate
// it; a local-scope request activates only on a fresh device.
makeDefault: !isLocalScope || shouldPromoteLocalToPrimary,
});
return NextResponse.json({ success: true });
}
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/setup-api/ai-models/configure/route.ts` around lines 702 - 722,
Update the Branch A condition in the openclawIsAbsent() flow to recognize local
providers by provider identity, not only isLocalScope: route llama.cpp and
Ollama requests through the existing local configuration and
applyLocalAiToHermes logic even when scope is "primary". Preserve the current
shouldPromoteLocalToPrimary behavior and model normalization, while leaving
non-local providers on their existing branches.

Comment on lines +709 to +720
await setMany({
local_ai_configured: true,
local_ai_provider: ocProvider,
local_ai_model: config.defaultModel,
local_ai_configured_at: new Date().toISOString(),
});
await applyLocalAiToHermes({
provider: ocProvider as "llamacpp" | "ollama",
// Hermes wants the bare model id, not the `llamacpp/…` qualified form.
model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
makeDefault: shouldPromoteLocalToPrimary,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ—„οΈ Data Integrity & Integration | 🟠 Major | ⚑ Quick win

Persist the local-model state only after Hermes accepts it.

setMany marks local_ai_configured: true before applyLocalAiToHermes runs. If the Hermes CLI call fails, the route returns 502 while the config store already reports the local model as configured. Hermes has no matching provider block at that point, so the status route reports a model that the device cannot serve.

applyCloudProviderKeyToHermes in src/lib/hermes-cloud-provider.ts (Lines 121-127) uses the opposite order: it writes ai_model_configured only after activation succeeds. Apply the same order here.

πŸ› Proposed reorder
-          await setMany({
-            local_ai_configured: true,
-            local_ai_provider: ocProvider,
-            local_ai_model: config.defaultModel,
-            local_ai_configured_at: new Date().toISOString(),
-          });
           await applyLocalAiToHermes({
             provider: ocProvider as "llamacpp" | "ollama",
             // Hermes wants the bare model id, not the `llamacpp/…` qualified form.
             model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
             makeDefault: shouldPromoteLocalToPrimary,
           });
+          await setMany({
+            local_ai_configured: true,
+            local_ai_provider: ocProvider,
+            local_ai_model: config.defaultModel,
+            local_ai_configured_at: new Date().toISOString(),
+          });
           return NextResponse.json({ success: true });
πŸ“ 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
await setMany({
local_ai_configured: true,
local_ai_provider: ocProvider,
local_ai_model: config.defaultModel,
local_ai_configured_at: new Date().toISOString(),
});
await applyLocalAiToHermes({
provider: ocProvider as "llamacpp" | "ollama",
// Hermes wants the bare model id, not the `llamacpp/…` qualified form.
model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
makeDefault: shouldPromoteLocalToPrimary,
});
await applyLocalAiToHermes({
provider: ocProvider as "llamacpp" | "ollama",
// Hermes wants the bare model id, not the `llamacpp/…` qualified form.
model: config.defaultModel.replace(/^(?:llamacpp|ollama)\//, ""),
makeDefault: shouldPromoteLocalToPrimary,
});
await setMany({
local_ai_configured: true,
local_ai_provider: ocProvider,
local_ai_model: config.defaultModel,
local_ai_configured_at: new Date().toISOString(),
});
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/setup-api/ai-models/configure/route.ts` around lines 709 - 720,
Reorder the local-model setup around applyLocalAiToHermes so Hermes activation
succeeds before persisting local_ai_configured, local_ai_provider,
local_ai_model, and local_ai_configured_at via setMany. Keep the existing
provider and model normalization, then perform the state write only after
successful activation, matching the ordering used by
applyCloudProviderKeyToHermes.

Comment on lines +735 to +741
if (result.activated) {
return NextResponse.json({ success: true });
}
return NextResponse.json(
{ error: "Key saved. Open the Hermes provider panel to pick a model for it." },
{ status: 409 },
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ—„οΈ Data Integrity & Integration | πŸ”΅ Trivial | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the client code that posts to the configure route and inspect its status handling.
rg -nP -C 10 'ai-models/configure' src/components/ src/hooks/ src/app/ -g '!**/route.ts'

Repository: ID-Robots/clawbox

Length of output: 4835


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- route Hermes branch and error mapping ---'
sed -n '680,770p' src/app/setup-api/ai-models/configure/route.ts
printf '%s\n' '--- Hermes helper definitions and error classes ---'
rg -n -P -C 12 'class Hermes(Local|Cloud)ApplyError|apply(LocalAi|Clawai|CloudProviderKey)ToHermes|Hermes.*ApplyError' src
printf '%s\n' '--- client status and error handling ---'
rg -n -P -C 18 'showError|extractError|selectedProvider|provider.*hermes|hermes' src/components/AIModelsStep.tsx

Repository: ID-Robots/clawbox

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- Hermes helper error behavior ---'
sed -n '1,180p' src/lib/hermes-local-ai.ts
sed -n '1,150p' src/lib/hermes-cloud-provider.ts
sed -n '1,130p' src/lib/hermes-clawai.ts
printf '%s\n' '--- wizard response handling ---'
sed -n '930,990p' src/components/AIModelsStep.tsx
printf '%s\n' '--- route tests for the disputed responses ---'
rg -n -P -C 10 '409|502|activated|saved|ApplyError|status\)' src/tests/routes/ai-models/configure-hermes.test.ts

Repository: ID-Robots/clawbox

Length of output: 25599


🏁 Script executed:

#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path

client = Path("src/components/AIModelsStep.tsx").read_text()
route = Path("src/app/setup-api/ai-models/configure/route.ts").read_text()
local = Path("src/lib/hermes-local-ai.ts").read_text()
cloud = Path("src/lib/hermes-cloud-provider.ts").read_text()
clawai = Path("src/lib/hermes-clawai.ts").read_text()

assert 'if (!res.ok) return showError(await extractError(res, "Failed to configure"));' in client
assert 'showSuccessAndContinue();' in client
assert '{ status: 409 }' in route
assert '{ error: err.message }, { status: 502 }' in route
assert 'Local model id is missing or malformed.' in local
assert 'An API key is required.' in cloud
assert 'class HermesLocalApplyError extends Error {}' in local
assert 'class HermesCloudApplyError extends Error {' in cloud
assert 'class ClawaiApplyError extends Error {}' in clawai

print("wizard treats every non-2xx configure response as an error")
print("configure route returns 409 for stored-but-unactivated keys")
print("configure route returns 502 for all caught Hermes helper errors")
print("validation messages share helper error classes with CLI failure messages")
PY

Repository: ID-Robots/clawbox

Length of output: 411


Align Hermes response semantics with the wizard.

  • Return a success response for a stored but unactivated key, or handle 409 explicitly in AIModelsStep.tsx. The wizard treats every non-2xx response as an error, so it displays the partial-success message as a failure and does not continue.
  • Add an error discriminator to the Hermes error classes. Map validation errors, such as "Local model id is missing or malformed." and "An API key is required.", to 400; keep Hermes CLI failures at 502.
πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/setup-api/ai-models/configure/route.ts` around lines 735 - 741,
Update the Hermes setup flow around result.activated so stored but unactivated
keys return a 2xx success response, or explicitly handle the 409 continuation
path in AIModelsStep.tsx. Add an error discriminator to the Hermes error classes
and map validation failures, including missing or malformed local model IDs and
missing API keys, to HTTP 400 while preserving HTTP 502 for Hermes CLI failures.

Source: Path instructions

Comment on lines +314 to +315
const tokenRaw = await getConfigValue(CLAWBOX_AI_TOKEN_CONFIG_KEY).catch(() => null);
const clawaiToken = typeof tokenRaw === "string" && tokenRaw.startsWith("claw_") ? tokenRaw : 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.

πŸ”’ Security & Privacy | 🟠 Major | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 'CLAWBOX_AI_DEVICE_INFO_URL|PORTAL_DEVICE_INFO_URL|fetchPortalTier' src README.md
rg -n -C 4 'redirect:|Authorization: `Bearer' src/app/setup-api/ai-models/status/route.ts

Repository: ID-Robots/clawbox

Length of output: 4085


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

Reachability: Internal

Reachability path
● Entry
  src/tests/routes/ai-models/status.test.ts:47
β”‚
β–Ό
● Sink
  src/app/setup-api/ai-models/status/route.ts

Restrict portal-token requests to trusted HTTPS endpoints.

Validate CLAWBOX_AI_DEVICE_INFO_URL against an HTTPS origin allowlist before sending the Bearer token. Set redirect: "error" or validate every redirect destination.

πŸ€– Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/app/setup-api/ai-models/status/route.ts` around lines 314 - 315, Before
the portal-token request that uses clawaiToken, validate
CLAWBOX_AI_DEVICE_INFO_URL as a trusted HTTPS origin using an explicit
allowlist, and reject non-HTTPS or unapproved origins before sending the Bearer
token. Configure the request to use redirect: "error" so the token cannot follow
an unvalidated redirect.

Comment on lines +80 to +87
const add = await runHermesCli(
["auth", "add", slug, "--type", "api-key", "--api-key", key],
{ timeoutMs: 20_000 },
);
if (add.code !== 0) {
// Never echo the key. hermes stderr may name the provider but not the secret.
throw new HermesCloudApplyError(add.stderr || "Failed to save the API key.");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ”’ Security & Privacy | 🟠 Major | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- Hermes helper error construction ---'
rg -n -C 4 'stderr|class Hermes(Local|Cloud|Clawai)ApplyError|`@throws`' \
  src/lib/hermes-cloud-provider.ts src/lib/hermes-local-ai.ts src/lib/hermes-clawai.ts
printf '%s\n' '--- Configure route Hermes catch and call sites ---'
rg -n -C 12 'HermesCloudApplyError|HermesLocalApplyError|ClawaiApplyError|applyCloudProviderKeyToHermes|err\.message' \
  src/app/setup-api/ai-models/configure/route.ts
printf '%s\n' '--- Relevant imports and logger usage ---'
sed -n '1,145p' src/lib/hermes-cloud-provider.ts
sed -n '720,775p' src/app/setup-api/ai-models/configure/route.ts

Repository: ID-Robots/clawbox

Length of output: 22000


Information Disclosure (CWE-209): Generation of Error Message Containing Sensitive Information

Reachability path
● Entry
  src/tests/routes/ai-models/configure-hermes.test.ts:145
β”‚
β–Ό
● Hop
  src/app/setup-api/ai-models/configure/route.ts:731
  applyCloudProviderKeyToHermes
β”‚
β–Ό
● Sink
  src/lib/hermes-cloud-provider.ts

Return authored Hermes error messages. The configure route echoes Hermes helper errors to the client. Replace raw stderr in src/lib/hermes-cloud-provider.ts, src/lib/hermes-local-ai.ts, and src/lib/hermes-clawai.ts with authored messages. Log only redacted diagnostics server-side. Keep the route comment aligned with this guarantee.

πŸ“ Affects 2 files
  • src/lib/hermes-cloud-provider.ts#L80-L87 (this comment)
  • src/lib/hermes-cloud-provider.ts#L112-L119
  • src/app/setup-api/ai-models/configure/route.ts#L749-L757
πŸ€– 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-cloud-provider.ts` around lines 80 - 87, Replace raw Hermes
stderr with authored, secret-safe error messages in the failure paths around
runHermesCli in src/lib/hermes-cloud-provider.ts (lines 80-87 and 112-119),
src/lib/hermes-local-ai.ts, and src/lib/hermes-clawai.ts; log only redacted
diagnostics server-side. Update the configure route comment in
src/app/setup-api/ai-models/configure/route.ts (lines 749-757) to state that
client responses use authored messages rather than helper stderr.

Comment on lines +92 to +99
try {
const payload = await getModelOptions({ refresh: true });
if (isAllowedProvider(payload, slug)) {
model = (await scopeFromPayload(payload, slug)).defaultModel || "";
}
} catch {
// Catalog unreachable β€” treat as "no model yet" and leave the provider be.
}

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 | πŸ”΅ Trivial | ⚑ Quick win

Log the swallowed catalog error.

The catch block discards every error. The comment attributes it to an unreachable catalog, but the block also absorbs errors from isAllowedProvider and scopeFromPayload, including type errors. The function then returns activated: false, and the caller reports "Key saved. Open the Hermes provider panel to pick a model for it."

A code defect and a network timeout produce the same user-visible result with no server-side trace. The device is embedded hardware with no interactive debugger.

Add a warning log.

As per path instructions: "TypeScript server-side libraries. Review for proper error handling and type safety".

♻️ Proposed logging
-  } catch {
+  } catch (err) {
     // Catalog unreachable β€” treat as "no model yet" and leave the provider be.
+    console.warn(
+      "[hermes-cloud-provider] model resolution failed for",
+      slug,
+      ":",
+      err instanceof Error ? err.message : err,
+    );
   }
πŸ“ 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
try {
const payload = await getModelOptions({ refresh: true });
if (isAllowedProvider(payload, slug)) {
model = (await scopeFromPayload(payload, slug)).defaultModel || "";
}
} catch {
// Catalog unreachable β€” treat as "no model yet" and leave the provider be.
}
try {
const payload = await getModelOptions({ refresh: true });
if (isAllowedProvider(payload, slug)) {
model = (await scopeFromPayload(payload, slug)).defaultModel || "";
}
} catch (err) {
// Catalog unreachable β€” treat as "no model yet" and leave the provider be.
console.warn(
"[hermes-cloud-provider] model resolution failed for",
slug,
":",
err instanceof Error ? err.message : err,
);
}
πŸ€– 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-cloud-provider.ts` around lines 92 - 99, Update the catch
block in the model-option loading flow around getModelOptions,
isAllowedProvider, and scopeFromPayload to emit a warning containing the caught
error and relevant provider context before preserving the existing fallback
behavior. Use the module’s established logging mechanism and keep returning with
no selected model after logging.

Source: Path instructions

// the refresh must skip it cleanly rather than fork a missing binary once per
// provider on every boot warmup.

vi.mock("child_process", () => ({ spawn: vi.fn() }));

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
# Inspect fetchOpenclawCatalog to see how the spawn result is consumed.
ast-grep outline src/app/setup-api/ai-models/catalog/route.ts --items all
rg -n -C 15 'function fetchOpenclawCatalog' src/app/setup-api/ai-models/catalog/route.ts

Repository: ID-Robots/clawbox

Length of output: 4025


🏁 Script executed:

#!/bin/bash
sed -n '353,475p' src/app/setup-api/ai-models/catalog/route.ts
printf '\n--- test file ---\n'
cat -n src/tests/routes/ai-models/catalog-edition.test.ts
printf '\n--- related spawn mocks and refresh calls ---\n'
rg -n -C 8 'refreshInBackground|child_process|spawn' src/tests src/app/setup-api/ai-models/catalog/route.ts

Repository: ID-Robots/clawbox

Length of output: 50374


🏁 Script executed:

#!/bin/bash
printf '%s\n' '--- module tail ---'
sed -n '470,545p' src/app/setup-api/ai-models/catalog/route.ts
printf '%s\n' '--- test configuration ---'
fd -i 'package.json|vitest.config.*|vite.config.*' . -t f | head -20
for f in $(fd -i 'package.json|vitest.config.*|vite.config.*' . -t f | head -20); do
  echo "--- $f"
  sed -n '1,180p' "$f"
done
printf '%s\n' '--- Promise executor behavior probe ---'
node - <<'JS'
const spawn = () => undefined;
let timerFired = false;
function fetchCatalog() {
  return new Promise((resolve, reject) => {
    const child = spawn();
    const timer = setTimeout(() => { timerFired = true; }, 300);
    child.stdout.on("data", () => {});
    resolve();
  });
}
let caught = false;
fetchCatalog().catch(() => { caught = true; });
setTimeout(() => {
  console.log(JSON.stringify({ caught, timerFired }));
  process.exit(0);
}, 50);
JS

Repository: ID-Robots/clawbox

Length of output: 8777


Return a closing child-process stub from the mock. The synchronous child.stdout access rejects the Promise and reaches .catch(). However, the five-minute timeout starts before that access and remains pending. Return a stub with the required stream and event methods, then emit close asynchronously to clear the timeout.

πŸ€– 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/ai-models/catalog-edition.test.ts` at line 9, Update the
child_process mock’s spawn stub to return a child-process-shaped object with the
required stdout stream and event-listener methods, and asynchronously emit close
so the pending timeout is cleared. Preserve the existing mock behavior while
ensuring synchronous child.stdout access succeeds and the Promise settles
through the close path.

Comment on lines +155 to +169
it("switches the local model (Gemma) through Hermes without spawning openclaw", async () => {
const res = await POST(jsonRequest({ provider: "llamacpp", scope: "local" }));
const body = await res.json();

expect(res.status).toBe(200);
expect(body.success).toBe(true);
expect(mockApplyLocalAiToHermes).toHaveBeenCalledWith(
expect.objectContaining({ provider: "llamacpp", model: "gemma4-e2b-it-q4_0", makeDefault: true }),
);
expect(mockSetMany).toHaveBeenCalledWith(expect.objectContaining({
local_ai_configured: true,
local_ai_provider: "llamacpp",
}));
expectNoOpenclawSpawn();
});

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 | πŸ”΅ Trivial | ⚑ Quick win

Add coverage for a local provider with the default primary scope.

Every local-model test in this suite passes scope: "local". The route's Hermes branch at src/app/setup-api/ai-models/configure/route.ts Line 704 gates on isLocalScope, so a request such as { provider: "llamacpp", apiKey: "gemma-q4" } with the default primary scope never reaches applyLocalAiToHermes. configure.test.ts Line 665 already posts that exact shape against the OpenClaw path.

Add a test that posts a llama.cpp request without scope and asserts applyLocalAiToHermes is called. That test pins the branch-gate fix.

πŸ€– 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/ai-models/configure-hermes.test.ts` around lines 155 - 169,
Add a test in the local-model suite that posts a llama.cpp configuration without
the scope field, using the default primary scope request shape, and assert the
response succeeds and mockApplyLocalAiToHermes is called. Keep the existing
explicit scope: "local" coverage unchanged and retain the no-OpenClaw assertion
for this default-scope case.

Comment on lines +71 to +75
// Edition guard: these tests exercise the OpenClaw path, so openclaw is
// present. The Hermes branch (openclawIsAbsent β†’ true) is covered separately
// in configure-hermes.test.ts.
openclawIsAbsent: vi.fn().mockReturnValue(false),
OpenclawUnavailableError: class OpenclawUnavailableError extends Error {},

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 | 🟑 Minor | ⚑ Quick win

openclawIsAbsent relies on a factory default that vi.clearAllMocks() removes. Both suites set mockReturnValue(false) inside the vi.mock("@/lib/openclaw-config", …) factory, and both call vi.clearAllMocks() in beforeEach. The mock therefore returns undefined at test time. undefined is falsy, so the OpenClaw path is still taken and both suites pass, but the intent is not enforced. Re-seed the value in beforeEach at each site.

  • src/tests/routes/ai-models/configure.test.ts#L71-L75: add vi.mocked(openclawIsAbsent).mockReturnValue(false); to the existing re-seed block at Lines 193-207, and import openclawIsAbsent on Line 109.
  • src/tests/routes/local-ai.test.ts#L17-L19: add the same re-seed to beforeEach after Line 76, and import openclawIsAbsent on Line 28.
πŸ“ Affects 2 files
  • src/tests/routes/ai-models/configure.test.ts#L71-L75 (this comment)
  • src/tests/routes/local-ai.test.ts#L17-L19
πŸ€– 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/ai-models/configure.test.ts` around lines 71 - 75, Re-seed
openclawIsAbsent after vi.clearAllMocks() in both test suites: import it in
src/tests/routes/ai-models/configure.test.ts and
src/tests/routes/local-ai.test.ts, then add
vi.mocked(openclawIsAbsent).mockReturnValue(false) to each suite’s beforeEach
re-seed block. Apply the corresponding changes at
src/tests/routes/ai-models/configure.test.ts lines 71-75 and
src/tests/routes/local-ai.test.ts lines 17-19; both sites require the same fix.

Comment on lines +123 to +128
it("refuses `openclaw config set` with a typed error on hermes instead of spawning", async () => {
process.env.CLAWBOX_EDITION = "hermes";
await expect(
openclawConfig.runOpenclawConfigSet(["model.provider", "clawai"]),
).rejects.toBeInstanceOf(openclawConfig.OpenclawUnavailableError);
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟑 Minor | ⚑ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/tests/unit/openclaw-config.test.ts --items all
rg -n -C 5 'spawn|child_process|runOpenclawConfigSet|edition guard' src/tests/unit/openclaw-config.test.ts

Repository: ID-Robots/clawbox

Length of output: 1916


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- test setup and target test ---'
sed -n '1,140p' src/tests/unit/openclaw-config.test.ts

printf '%s\n' '--- implementation references ---'
rg -n -C 8 'runOpenclawConfigSet|OpenclawUnavailableError|openclawIsAbsent|execFile|spawn' src

Repository: ID-Robots/clawbox

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

test_file="$(fd -t f -i '^openclaw-config\.test\.ts$' | head -n 1)"
config_file="$(fd -t f -i '^openclaw-config\.ts$' | head -n 1)"

printf '%s\n' "--- ${test_file} ---"
sed -n '1,135p' "$test_file"

printf '%s\n' "--- ${config_file} ---"
rg -n -C 12 'runOpenclawConfigSet|OpenclawUnavailableError|openclawIsAbsent|execFile|spawn' "$config_file"

Repository: ID-Robots/clawbox

Length of output: 16120


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

test_path = Path("src/tests/unit/openclaw-config.test.ts")
source_path = Path("src/lib/openclaw-config.ts")

test = test_path.read_text()
source = source_path.read_text()

mock = re.search(r'vi\.mock\("child_process",\s*\(\)\s*=>\s*\(\{(.*?)\}\)\);', test, re.S)
spawn_guard = re.search(
    r'if \(openclawIsAbsent\(\)\)\s*\{\s*return Promise\.reject\(new OpenclawUnavailableError\(\)\);\s*\}\s*.*?const child = spawn\(',
    source,
    re.S,
)

print("test child_process mock defines spawn:", bool(mock and re.search(r'\bspawn\s*:', mock.group(1))))
print("test child_process mock defines execFile:", bool(mock and re.search(r'\bexecFile\s*:', mock.group(1))))
print("test target observes mockExecFile:", "mockExecFile" in test)
print("implementation checks edition before spawn:", bool(spawn_guard))
PY

Repository: ID-Robots/clawbox

Length of output: 336


Assert that Hermes does not invoke child_process.spawn.

The test mocks only execFile, but runOpenclawConfigSet uses spawn. Add spawn: vi.fn() to the mock and assert zero calls after the OpenclawUnavailableError rejection.

πŸ€– 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/unit/openclaw-config.test.ts` around lines 123 - 128, Add spawn:
vi.fn() to the child_process mock used by the openclawConfig tests, then update
the Hermes runOpenclawConfigSet test to assert spawn was not called after the
OpenclawUnavailableError rejection. Keep the existing execFile mock and
typed-error assertion unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: gateway Auto-triage area area: ui Auto-triage area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant