Add OpenClaw gateway provider#65
Merged
Merged
Conversation
|
Looking for one thing? Review this PR in Change Stack to search files, summaries, diffs, and code without losing your place. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds OpenClaw as a first-class provider across contracts, server (protocol/client/adapter/secrets/health/settings/RPC), runtime wiring, and web (settings/UI/composer). Introduces gateway protocol, URL normalization/redaction, server-owned secret lifecycle and RPC, adapter event translation, fixed model sentinel ("gateway"), and comprehensive tests and docs. ChangesOpenClaw Provider End-to-End Integration
Sequence Diagram(s)sequenceDiagram
participant Web as Web (Settings)
participant WS as WS RPC
participant Secret as SecretStore
participant Settings as Server Settings
Web->>WS: server.updateOpenClawSecrets(token/password/device ops)
WS->>Secret: applyOpenClawSecretUpdate(...)
Secret-->>WS: OpenClawSecretMetadata (hasToken/hasPassword/paired)
WS->>Settings: updateOpenClawSecretMetadata(metadata)
Settings-->>Web: updated settings + status
sequenceDiagram
participant Client as Client (Chat)
participant Adapter as Server Adapter (OpenClaw)
participant Gateway as Gateway WS
Client->>Adapter: startSession(provider: openclaw, model: gateway)
Adapter->>Gateway: connect (+ optional challenge response)
Gateway-->>Adapter: methods, protocolVersion
Adapter->>Gateway: chat.history
Client->>Adapter: sendTurn(text)
Adapter->>Gateway: chat.send(sessionKey, message, idempotencyKey)
Gateway-->>Adapter: content.delta ... run.completed / error
Adapter-->>Client: canonical runtime events (content.delta, item.completed, turn.completed, runtime.error)
Estimated code review effort Possibly related PRs
|
github-actions
Bot
added
size:XXL
vouch:trusted
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
docs/adr/README.md (1)
11-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate ADR index metadata review date.
After adding ADR
0005, the metadata table still showsLast reviewedas2026-05-22(Line 11). Please bump it to the current review date to keep index metadata current.As per coding guidelines, "Keep metadata tables current when adding or substantially changing a document."
Also applies to: 24-24
🤖 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 `@docs/adr/README.md` around lines 11 - 12, Update the ADR index metadata "Last reviewed" entry in docs/adr/README.md from "2026-05-22" to the current review date (e.g., 2026-06-06) so the table row with the "Last reviewed" header reflects the addition of ADR `0005`; locate the "Last reviewed" cell in the metadata table and replace the old date string with the new review date.Source: Coding guidelines
apps/web/src/routes/_chat.settings.tsx (1)
618-629:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTrack
openClawEnabledeverywhere provider-tool state is derived.Right now flipping only the OpenClaw enable switch never marks Provider tools as dirty, never shows the row as custom, and neither reset path restores the default enabled state. After "Restore defaults", OpenClaw can stay disabled.
Proposed fix
const [openInstallProviders, setOpenInstallProviders] = useState<Record<ProviderKind, boolean>>({ @@ - openclaw: Boolean(settings.openClawGatewayUrl || settings.openClawAuthMode !== "none"), + openclaw: Boolean( + settings.openClawEnabled !== defaults.openClawEnabled || + settings.openClawGatewayUrl || + settings.openClawAuthMode !== "none", + ), @@ - : providerSettings.provider === "openclaw" - ? settings.openClawGatewayUrl !== defaults.openClawGatewayUrl || - settings.openClawAuthMode !== defaults.openClawAuthMode + : providerSettings.provider === "openclaw" + ? settings.openClawEnabled !== defaults.openClawEnabled || + settings.openClawGatewayUrl !== defaults.openClawGatewayUrl || + settings.openClawAuthMode !== defaults.openClawAuthMode : settings.piBinaryPath !== defaults.piBinaryPath || settings.piAgentDir !== defaults.piAgentDir, @@ updateSettings({ @@ + openClawEnabled: defaults.openClawEnabled, openClawAuthMode: defaults.openClawAuthMode, openClawGatewayUrl: defaults.openClawGatewayUrl, @@ - : providerSettings.provider === "openclaw" - ? settings.openClawGatewayUrl !== defaults.openClawGatewayUrl || - settings.openClawAuthMode !== defaults.openClawAuthMode + : providerSettings.provider === "openclaw" + ? settings.openClawEnabled !== defaults.openClawEnabled || + settings.openClawGatewayUrl !== defaults.openClawGatewayUrl || + settings.openClawAuthMode !== defaults.openClawAuthMode : settings.piBinaryPath !== defaults.piBinaryPath || settings.piAgentDir !== defaults.piAgentDir;Also applies to: 825-850, 2768-2800, 2810-2835
🤖 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 `@apps/web/src/routes/_chat.settings.tsx` around lines 618 - 629, The provider-tool derived state for OpenClaw is missing the explicit enabled flag; update all initializations and derivations of openInstallProviders (and similar provider-tool dirty/custom/reset logic) to include settings.openClawEnabled when computing the openclaw boolean (e.g., in the openInstallProviders useState initializer and the other occurrences noted). Concretely, change the openclaw expression to consider settings.openClawEnabled OR the existing heuristic (settings.openClawGatewayUrl || settings.openClawAuthMode !== "none"), ensure any dirty/custom checks and the "restore defaults" reset include openClawEnabled so toggling the enable switch marks tools dirty and restores correctly, and mirror this same change in the other identified blocks (the other initializers/derivations for provider tools).apps/web/src/appSettings.ts (2)
289-304:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize persisted git-writing providers into the new allowlist.
You narrowed git text generation to
codex/kilo/opencode, but stored values are still passed through unchanged. Any existing install that already hastextGenerationProviderset to something else will now carry an invalid selection with no matching option until the user manually fixes it.Proposed fix
function normalizeAppSettings(settings: AppSettings): AppSettings { + const textGenerationProvider = resolveTextGenerationProvider({ + provider: settings.textGenerationProvider, + model: settings.textGenerationModel, + }); + return { ...settings, chatFontSizePx: normalizeChatFontSizePx(settings.chatFontSizePx), customCodexModels: normalizeCustomModelSlugs(settings.customCodexModels, "codex"), customClaudeModels: normalizeCustomModelSlugs(settings.customClaudeModels, "claudeAgent"), customCursorModels: normalizeCustomModelSlugs(settings.customCursorModels, "cursor"), customGeminiModels: normalizeCustomModelSlugs(settings.customGeminiModels, "gemini"), customKiloModels: normalizeCustomModelSlugs(settings.customKiloModels, "kilo"), customOpenCodeModels: normalizeCustomModelSlugs(settings.customOpenCodeModels, "opencode"), customPiModels: normalizeCustomModelSlugs(settings.customPiModels, "pi"), + textGenerationProvider, hiddenProviders: normalizeHiddenProviders(settings.hiddenProviders), providerOrder: normalizeProviderOrder(settings.providerOrder), hiddenModels: [], }; }Also applies to: 343-352, 666-674
🤖 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 `@apps/web/src/appSettings.ts` around lines 289 - 304, Stored textGenerationProvider values can be outside the new allowlist (codex/kilo/opencode), causing invalid selections; update normalizeAppSettings to map/validate settings.textGenerationProvider into the new allowlist (e.g., if value is one of "codex","kilo","opencode" keep it, otherwise set to a safe default or null) before returning the settings, and add or reuse a helper (e.g., normalizeTextGenerationProvider) to perform this validation; apply the same mapping/validation in the other normalize blocks referenced (the similar normalize code at the other ranges) so persisted non-allowed providers are normalized consistently.
714-797:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
getProviderStartOptions()still drops the OpenClaw bootstrap config.
openClawGatewayUrlis part of the input type now, but this helper never emits anopenclawentry. Any caller that bootstraps provider startup from local app settings will silently start without the configured gateway URL.Proposed fix
const providerOptions: ProviderStartOptions = { @@ ...(settings.openCodeBinaryPath || settings.openCodeServerUrl || settings.openCodeServerPassword ? { opencode: { ...(settings.openCodeBinaryPath ? { binaryPath: settings.openCodeBinaryPath } : {}), ...(settings.openCodeServerUrl ? { serverUrl: settings.openCodeServerUrl } : {}), ...(settings.openCodeServerPassword ? { serverPassword: settings.openCodeServerPassword } : {}), }, } : {}), + ...(settings.openClawGatewayUrl + ? { + openclaw: { + gatewayUrl: settings.openClawGatewayUrl, + }, + } + : {}), ...(settings.piBinaryPath || settings.piAgentDir ? { pi: { ...(settings.piBinaryPath ? { binaryPath: settings.piBinaryPath } : {}), ...(settings.piAgentDir ? { agentDir: settings.piAgentDir } : {}),🤖 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 `@apps/web/src/appSettings.ts` around lines 714 - 797, getProviderStartOptions currently ignores the openClawGatewayUrl input so it never emits an openclaw provider entry; update getProviderStartOptions to include an openclaw key when settings.openClawGatewayUrl is present (e.g., produce { openclaw: { gatewayUrl: settings.openClawGatewayUrl } }) so callers bootstrapping providers from local settings pick up the configured gateway URL; modify the providerOptions construction near the opencode/pi blocks to add this conditional openclaw object keyed "openclaw" and return it as part of providerOptions.apps/web/src/composerDraftStore.ts (1)
1220-1222:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInclude
openclawin draft-provider fallback detection.When
activeProvideris null, this scan currently skipsopenclaw, so a draft that only has an OpenClaw selection can fall through to the wrong provider.Proposed fix
const draftProviderWithSelection = - (["codex", "claudeAgent", "cursor", "gemini", "kilo", "opencode", "pi"] as const).find( + ([ + "codex", + "claudeAgent", + "cursor", + "gemini", + "kilo", + "opencode", + "openclaw", + "pi", + ] as const).find( (provider) => input.draft?.modelSelectionByProvider?.[provider] !== undefined, ) ?? 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 `@apps/web/src/composerDraftStore.ts` around lines 1220 - 1222, The fallback scan that computes activeProvider currently checks the tuple ("codex","claudeAgent","cursor","gemini","kilo","opencode","pi") and omits "openclaw", so drafts with only an OpenClaw selection are missed; update that tuple in the expression that reads input.draft?.modelSelectionByProvider (the same expression used to set activeProvider) to include "openclaw" among the providers so the find(...) will detect OpenClaw selections as a valid fallback.apps/web/src/store.ts (1)
1931-1944:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
toLegacyProvidermust handle "openclaw" to prevent session corruption.The
toLegacyProviderfunction does not include "openclaw" in its recognized provider list. When the server sends a session withproviderName: "openclaw", this function will incorrectly return "codex" instead (line 1943 fallback).This causes a critical inconsistency:
- Thread
modelSelection.providerwill be "openclaw" (correctly normalized at lines 570-573)- Thread
session.providerwill be "codex" (incorrectly normalized here)Impact: OpenClaw sessions will be misidentified as Codex sessions throughout the UI and business logic, breaking the OpenClaw feature completely.
🔧 Proposed fix
function toLegacyProvider(providerName: string | null): ProviderKind { if ( providerName === "codex" || providerName === "claudeAgent" || providerName === "cursor" || providerName === "gemini" || providerName === "kilo" || + providerName === "openclaw" || providerName === "opencode" || providerName === "pi" ) { return providerName; } return "codex"; }🤖 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 `@apps/web/src/store.ts` around lines 1931 - 1944, The toLegacyProvider function currently maps unknown providerName values to "codex" causing "openclaw" sessions to be miscategorized; update toLegacyProvider (the function taking providerName: string | null and returning ProviderKind) to include "openclaw" in the allowed provider list alongside "codex", "claudeAgent", "cursor", "gemini", "kilo", "opencode", and "pi" so that when providerName === "openclaw" it returns "openclaw" (ensuring session.provider matches the normalized modelSelection.provider).apps/web/src/lib/threadHandoff.ts (1)
150-173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce OpenClaw’s fixed
gatewaymodel for all selection sources (Line 156).
isCompatibleSelectioncurrently accepts anyopenclawmodel, so stale sticky/project defaults can bypass the fixed gateway invariant.🔧 Suggested fix
export function resolveThreadHandoffModelSelection(input: { @@ }): ModelSelection { + if (input.targetProvider === "openclaw") { + return buildModelSelection("openclaw", "gateway"); + } + const isCompatibleSelection = ( selection: ModelSelection | null | undefined, ): selection is ModelSelection => { if (!selection || selection.provider !== input.targetProvider) { return false; } - return input.targetProvider !== "kilo" || selection.model.startsWith("kilo/"); + if (input.targetProvider === "kilo") { + return selection.model.startsWith("kilo/"); + } + return true; }; @@ const defaultModel = getDefaultModel(input.targetProvider); if (!defaultModel) { - if (input.targetProvider === "openclaw") { - return buildModelSelection("openclaw", "gateway"); - } throw new Error("Select a Pi model before handing off to Pi."); } return buildModelSelection(input.targetProvider, defaultModel); }🤖 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 `@apps/web/src/lib/threadHandoff.ts` around lines 150 - 173, The helper isCompatibleSelection currently permits any openclaw model, letting stale stickySelection or projectDefaultModelSelection bypass the invariant; update isCompatibleSelection to explicitly require selection.model === "gateway" when selection.provider or input.targetProvider is "openclaw" (i.e., return false for openclaw selections unless model === "gateway"), leaving the existing kilo check (selection.model.startsWith("kilo/")) intact; this ensures stickySelection and projectDefaultModelSelection are rejected for openclaw unless they are the fixed "gateway" model, and the existing fallback logic using getDefaultModel and buildModelSelection remains correct.
🤖 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 `@apps/server/src/provider/Layers/OpenClawAdapter.ts`:
- Around line 470-524: sendTurn can return without emitting a terminal
turn.completed when requestOverSocket returns a firstResult that contains only
runId (no events) because sendResultEvents([]) emits nothing; fix by ensuring we
treat an ack-only response as non-terminal: either modify requestOverSocket (the
firstResult/firstResult.runId logic) to continue reading until a run.completed
or error event is received, or update sendTurn/sendResultEvents to detect a
firstResult with runId and schedule/emulate a terminal turn.completed when later
results arrive (or when a timeout/ack indicates failure). Update the logic
around requestOverSocket, firstResult, sendResultEvents, and sendTurn
accordingly and add a regression test covering the gateway response that returns
only { runId } to assert the turn always emits a terminal turn.completed
(completed or failed).
In `@apps/server/src/provider/Layers/ProviderHealth.ts`:
- Around line 1541-1550: The current failure branch in ProviderHealth (when
Result.isFailure(probe) in the OpenClaw path) always reports a generic "gateway
is unreachable" message and discards probe error details; update the failure
handling in the OpenClaw probe path to inspect the probe error payload (e.g.,
check probe.error or probe.value for auth vs network errors) and set
status/message/authStatus accordingly instead of unconditionally returning the
unreachable message (refer to OPENCLAW_PROVIDER, Result.isFailure(probe), and
normalized.redactedUrl to locate the code). Add a focused regression test in
apps/server/src/provider/Layers/ProviderHealth.test.ts that exercises both an
auth rejection response and a network/unreachable response from the OpenClaw
probe and asserts that the returned ServerProviderStatus distinguishes
authStatus/messages for auth failures versus network unreachability.
In `@apps/server/src/provider/openclawGatewayClient.ts`:
- Around line 348-355: The current early-return checks in this block wrongly
treat presence of runId as a terminal condition, causing chat.send aggregation
to stop after a single delta; update the branch that checks firstResult so it
only returns immediately for full responses (e.g., when "events" is present) and
not simply because runId exists. Concretely, modify the condition around
normalizeRequestResult/firstResult (and the surrounding logic that uses
isGatewayEvent(firstFrame)) to remove the "runId" short-circuit and ensure
aggregation continues for gateway event frames until a terminal event like
run.completed or error is received; locate and change the logic in the function
using receiveFrame, normalizeRequestResult, firstFrame, and isGatewayEvent to
implement this behavior.
In `@apps/server/src/provider/openclawGatewayProtocol.ts`:
- Around line 165-174: The challengeSigningPayload function is building the
payload with literal backslash-n sequences because it uses "\\n"; change it to
use an actual newline character separator (e.g., '\n') when joining the array in
challengeSigningPayload so the string contains real newline bytes matching what
connect.challenge-response verification expects, and keep the same fields:
OPENCLAW_CLIENT_ID, OPENCLAW_CLIENT_MODE, OPENCLAW_CLIENT_DISPLAY_NAME,
input.deviceId, input.challenge.nonce, input.challenge.timestamp.
In `@apps/server/src/provider/openclawGatewayUrl.ts`:
- Around line 25-29: The code uses normalized.split(".")[0] which can be typed
as string | undefined under noUncheckedIndexedAccess; change the access to
provide a default so TypeScript knows it's a string: after the
Net.isIP(normalized) check, destructure with a default like const [firstOctetStr
= ""] = normalized.split("."); then use Number(firstOctetStr) (e.g., const
firstOctet = Number(firstOctetStr)) and return firstOctet === 127; this keeps
the runtime behavior and satisfies the compiler for symbols referencing
normalized and the first-octet computation.
In `@apps/server/src/serverSettings.test.ts`:
- Around line 190-192: Replace the manual string slice used to derive the
directory for settingsPath with cross-platform path utilities: import Node's
path module (e.g., import path from "path") and call path.dirname(settingsPath)
in the fs.makeDirectory call instead of settingsPath.slice(0,
settingsPath.lastIndexOf("/")); update the code in serverSettings.test.ts where
settingsPath is used so the test setup uses path.dirname(settingsPath) to
produce a platform-independent directory path.
In `@apps/web/src/lib/threadHandoff.test.ts`:
- Around line 129-146: Add a regression test in
apps/web/src/lib/threadHandoff.test.ts that verifies
resolveThreadHandoffModelSelection ignores non-"gateway" OpenClaw selections:
call resolveThreadHandoffModelSelection with sourceThread.modelSelection set
(e.g., provider "gemini"), targetProvider "openclaw", and then (a)
projectDefaultModelSelection = { provider: "openclaw", model: "openclaw-special"
} and (b) stickyModelSelectionByProvider = { openclaw: { provider: "openclaw",
model: "openclaw-special" } } (separate assertions or a combined table-driven
set) and assert the result is always { provider: "openclaw", model: "gateway" }
to ensure non-"gateway" defaults are ignored.
In `@apps/web/src/routes/-_chat.settings.install.test.ts`:
- Around line 5-8: The slice of settingsRouteSource into
defaultProviderSectionSource relies on two markers and can silently produce
wrong slices if a marker is missing or out of order; before calling slice,
compute const start = settingsRouteSource.indexOf('title="Default provider"')
and const end = settingsRouteSource.indexOf('title="New threads"'), then guard
by asserting or throwing if start === -1 || end === -1 || start >= end so the
test fails fast with a clear message; update the test to use these guarded
start/end values when creating defaultProviderSectionSource and include a
descriptive error string mentioning the missing or misordered markers.
In `@docs/superpowers/specs/2026-06-05-openclaw-provider-design.md`:
- Around line 187-188: Update the sentence that currently claims "The OpenClaw
secret store is a new provider-secret abstraction, not an existing JCode
service" to state instead that OpenClaw reuses the existing server secret-store
abstraction (do not imply a parallel backend), while preserving the
implementation requirements: create secretsDir when needed, write atomically,
use owner-only permissions where supported, and expose explicit clear/rotate
behavior for auth-mode changes; keep the settings read/write contract (expose
only redacted fields like authMode, hasSecret, paired; and use write-only secret
mutations such as set secret, clear secret, and rotate device identity) and
ensure the doc does not invent new commands or runtime behavior.
In `@packages/shared/src/serverSettings.ts`:
- Around line 22-24: The current conditional returns the original
whitespace-only string because it returns value before trimming; change the
logic so undefined is preserved but pure-whitespace strings are normalized to an
empty string — i.e., replace the block "if (value === undefined ||
value.trim().length === 0) { return value; }" with an explicit check that
returns value if value === undefined, and returns '' when value.trim().length
=== 0; apply the same change to the other identical occurrence in the file (the
second conditional at the other location).
---
Outside diff comments:
In `@apps/web/src/appSettings.ts`:
- Around line 289-304: Stored textGenerationProvider values can be outside the
new allowlist (codex/kilo/opencode), causing invalid selections; update
normalizeAppSettings to map/validate settings.textGenerationProvider into the
new allowlist (e.g., if value is one of "codex","kilo","opencode" keep it,
otherwise set to a safe default or null) before returning the settings, and add
or reuse a helper (e.g., normalizeTextGenerationProvider) to perform this
validation; apply the same mapping/validation in the other normalize blocks
referenced (the similar normalize code at the other ranges) so persisted
non-allowed providers are normalized consistently.
- Around line 714-797: getProviderStartOptions currently ignores the
openClawGatewayUrl input so it never emits an openclaw provider entry; update
getProviderStartOptions to include an openclaw key when
settings.openClawGatewayUrl is present (e.g., produce { openclaw: { gatewayUrl:
settings.openClawGatewayUrl } }) so callers bootstrapping providers from local
settings pick up the configured gateway URL; modify the providerOptions
construction near the opencode/pi blocks to add this conditional openclaw object
keyed "openclaw" and return it as part of providerOptions.
In `@apps/web/src/composerDraftStore.ts`:
- Around line 1220-1222: The fallback scan that computes activeProvider
currently checks the tuple
("codex","claudeAgent","cursor","gemini","kilo","opencode","pi") and omits
"openclaw", so drafts with only an OpenClaw selection are missed; update that
tuple in the expression that reads input.draft?.modelSelectionByProvider (the
same expression used to set activeProvider) to include "openclaw" among the
providers so the find(...) will detect OpenClaw selections as a valid fallback.
In `@apps/web/src/lib/threadHandoff.ts`:
- Around line 150-173: The helper isCompatibleSelection currently permits any
openclaw model, letting stale stickySelection or projectDefaultModelSelection
bypass the invariant; update isCompatibleSelection to explicitly require
selection.model === "gateway" when selection.provider or input.targetProvider is
"openclaw" (i.e., return false for openclaw selections unless model ===
"gateway"), leaving the existing kilo check
(selection.model.startsWith("kilo/")) intact; this ensures stickySelection and
projectDefaultModelSelection are rejected for openclaw unless they are the fixed
"gateway" model, and the existing fallback logic using getDefaultModel and
buildModelSelection remains correct.
In `@apps/web/src/routes/_chat.settings.tsx`:
- Around line 618-629: The provider-tool derived state for OpenClaw is missing
the explicit enabled flag; update all initializations and derivations of
openInstallProviders (and similar provider-tool dirty/custom/reset logic) to
include settings.openClawEnabled when computing the openclaw boolean (e.g., in
the openInstallProviders useState initializer and the other occurrences noted).
Concretely, change the openclaw expression to consider settings.openClawEnabled
OR the existing heuristic (settings.openClawGatewayUrl ||
settings.openClawAuthMode !== "none"), ensure any dirty/custom checks and the
"restore defaults" reset include openClawEnabled so toggling the enable switch
marks tools dirty and restores correctly, and mirror this same change in the
other identified blocks (the other initializers/derivations for provider tools).
In `@apps/web/src/store.ts`:
- Around line 1931-1944: The toLegacyProvider function currently maps unknown
providerName values to "codex" causing "openclaw" sessions to be miscategorized;
update toLegacyProvider (the function taking providerName: string | null and
returning ProviderKind) to include "openclaw" in the allowed provider list
alongside "codex", "claudeAgent", "cursor", "gemini", "kilo", "opencode", and
"pi" so that when providerName === "openclaw" it returns "openclaw" (ensuring
session.provider matches the normalized modelSelection.provider).
In `@docs/adr/README.md`:
- Around line 11-12: Update the ADR index metadata "Last reviewed" entry in
docs/adr/README.md from "2026-05-22" to the current review date (e.g.,
2026-06-06) so the table row with the "Last reviewed" header reflects the
addition of ADR `0005`; locate the "Last reviewed" cell in the metadata table
and replace the old date string with the new review date.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 85f0a89e-17ce-4664-a1cb-efcf908e7683
📒 Files selected for processing (77)
CONTEXT.mdapps/server/integration/orchestrationEngine.integration.test.tsapps/server/src/orchestration/Layers/ProviderCommandReactor.tsapps/server/src/orchestration/Layers/ProviderRuntimeIngestion.tsapps/server/src/provider/Layers/OpenClawAdapter.test.tsapps/server/src/provider/Layers/OpenClawAdapter.tsapps/server/src/provider/Layers/ProviderAdapterRegistry.test.tsapps/server/src/provider/Layers/ProviderAdapterRegistry.tsapps/server/src/provider/Layers/ProviderHealth.test.tsapps/server/src/provider/Layers/ProviderHealth.tsapps/server/src/provider/Services/OpenClawAdapter.tsapps/server/src/provider/openclawGatewayClient.test.tsapps/server/src/provider/openclawGatewayClient.tsapps/server/src/provider/openclawGatewayProtocol.test.tsapps/server/src/provider/openclawGatewayProtocol.tsapps/server/src/provider/openclawGatewayUrl.test.tsapps/server/src/provider/openclawGatewayUrl.tsapps/server/src/provider/openclawSecretUpdate.tsapps/server/src/provider/openclawSecrets.test.tsapps/server/src/provider/openclawSecrets.tsapps/server/src/provider/providerStatusCache.test.tsapps/server/src/provider/providerStatusCache.tsapps/server/src/provider/runtimeLayer.tsapps/server/src/serverSettings.test.tsapps/server/src/serverSettings.tsapps/server/src/wsRpc.tsapps/web/src/appSettings.test.tsapps/web/src/appSettings.tsapps/web/src/components/ChatView.tsxapps/web/src/components/PluginLibrary.tsxapps/web/src/components/Sidebar.tsxapps/web/src/components/SkillLibrarySettingsPanel.test.tsapps/web/src/components/SkillLibrarySettingsPanel.tsxapps/web/src/components/chat/CompactComposerControlsMenu.browser.tsxapps/web/src/components/chat/ProviderModelPicker.browser.tsxapps/web/src/components/chat/ProviderModelPicker.tsxapps/web/src/components/chat/TraitsPicker.browser.tsxapps/web/src/components/chat/composerProviderRegistry.tsxapps/web/src/composerDraftStore.test.tsapps/web/src/composerDraftStore.tsapps/web/src/hooks/useHandleNewThread.tsapps/web/src/lib/threadHandoff.test.tsapps/web/src/lib/threadHandoff.tsapps/web/src/providerModelOptions.tsapps/web/src/providerOrdering.test.tsapps/web/src/providerOrdering.tsapps/web/src/routes/-_chat.settings.install.test.tsapps/web/src/routes/_chat.settings.tsxapps/web/src/session-logic.test.tsapps/web/src/session-logic.tsapps/web/src/store.test.tsapps/web/src/store.tsapps/web/src/wsNativeApi.test.tsapps/web/src/wsNativeApi.tsdocs/adr/0005-openclaw-gateway-provider.mddocs/adr/README.mddocs/superpowers/plans/2026-06-05-openclaw-provider-implementation.mddocs/superpowers/specs/2026-06-05-openclaw-provider-design.mdpackages/contracts/src/agentMentions.tspackages/contracts/src/ipc.tspackages/contracts/src/model.tspackages/contracts/src/orchestration.test.tspackages/contracts/src/orchestration.tspackages/contracts/src/provider.test.tspackages/contracts/src/providerDiscovery.tspackages/contracts/src/providerRuntime.test.tspackages/contracts/src/providerRuntime.tspackages/contracts/src/rpc.tspackages/contracts/src/server.tspackages/contracts/src/settings.test.tspackages/contracts/src/settings.tspackages/contracts/src/ws.test.tspackages/contracts/src/ws.tspackages/shared/src/model.test.tspackages/shared/src/model.tspackages/shared/src/serverSettings.test.tspackages/shared/src/serverSettings.ts
# Conflicts: # apps/web/src/routes/_chat.settings.tsx # packages/contracts/src/ws.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/web/src/components/ChatView.tsx (2)
4018-4044:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-enable tail-follow when the user manually reaches the bottom again.
After
disableTailFollow()fires, scrolling back to the bottom only hides the CTA here; it never restorestailFollowEnabledRef. The next assistant message will stay off-screen until the user clicks the button or sends another turn.Suggested fix
- const onIsAtEndChange = useCallback((reportedIsAtEnd: boolean) => { + const onIsAtEndChange = useCallback((reportedIsAtEnd: boolean) => { let isAtEnd = reportedIsAtEnd; if (reportedIsAtEnd) { const scrollContainer = legendListRef.current?.getScrollableNode?.(); if (scrollContainer instanceof HTMLElement) { isAtEnd = isScrollContainerNearBottom({ @@ isAtEndRef.current = isAtEnd; if (isAtEnd) { - showScrollDebouncer.current.cancel(); - setShowScrollToBottom(false); + enableTailFollow(); } else if (tailFollowEnabledRef.current) { showScrollDebouncer.current.cancel(); setShowScrollToBottom(false); } else { showScrollDebouncer.current.maybeExecute(); } - }, []); + }, [enableTailFollow]);🤖 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 `@apps/web/src/components/ChatView.tsx` around lines 4018 - 4044, The onIsAtEndChange handler never re-enables tail-follow when the user manually scrolls back to the bottom; update the handler so that when isAtEnd becomes true you restore tail-follow (e.g., call the existing setter or update tailFollowEnabledRef.current = true — use setTailFollowEnabled(true) if that setter exists) in addition to cancelling debouncers and hiding the CTA; reference onIsAtEndChange and tailFollowEnabledRef (and setTailFollowEnabled if present) to locate where to add this restore logic.
5668-5699:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't silently downgrade a worktree draft to local execution.
This new branch converts a first-send
"worktree"draft into"local"before the base-branch validation runs. If branch discovery is still unresolved, the turn now executes in the main workspace instead of stopping with the expected error.Suggested fix
- if ( - isFirstMessage && - nextThreadEnvMode === "worktree" && - !nextThreadBranch && - !nextThreadWorktreePath - ) { - nextThreadEnvMode = "local"; - if (isLocalDraftThread) { - setDraftThreadContext(threadIdForSend, { - envMode: "local", - branch: null, - worktreePath: null, - }); - } - } - const baseBranchForWorktree = isFirstMessage && nextThreadEnvMode === "worktree" && !nextThreadWorktreePath ? nextThreadBranch : 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 `@apps/web/src/components/ChatView.tsx` around lines 5668 - 5699, The code currently mutates nextThreadEnvMode to "local" for first-message drafts (the block that sets nextThreadEnvMode = "local" and calls setDraftThreadContext) before validating that a base branch is present, which allows a missing branch to silently downgrade to local execution; revert that change so validation runs first: remove or relocate the early downgrade and setDraftThreadContext call from the start of the block and ensure the shouldCreateWorktree check (using isFirstMessage, nextThreadEnvMode, nextThreadWorktreePath, nextThreadBranch) runs while nextThreadEnvMode still equals "worktree"; only set envMode to "local" and call setDraftThreadContext (for isLocalDraftThread / threadIdForSend) after branch discovery is confirmed to have failed and after the validation error has not been triggered (or explicitly when you intend to convert the draft), and keep the setStoreThreadError(threadIdForSend, "Select a base branch...") return path intact.packages/contracts/src/providerDiscovery.ts (1)
109-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winModel OpenClaw gateway URL in the start-options contract.
Line [109] defines
OpenClawProviderStartOptionsas an empty struct, soproviderOptions.openclaw.gatewayUrlis outside the contract and is dropped at schema decode boundaries. That breaks the OpenClaw adapter option path.Proposed fix
-export const OpenClawProviderStartOptions = Schema.Struct({}); +export const OpenClawProviderStartOptions = Schema.Struct({ + gatewayUrl: Schema.optional(TrimmedNonEmptyString), +});Based on learnings from
apps/server/src/provider/Layers/OpenClawAdapter.test.ts:155, the adapter path expectsproviderOptions.openclaw.gatewayUrl.🤖 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 `@packages/contracts/src/providerDiscovery.ts` around lines 109 - 119, OpenClawProviderStartOptions is currently an empty Schema.Struct, so providerOptions.openclaw.gatewayUrl is dropped at decode boundaries; update the OpenClawProviderStartOptions definition to include a gatewayUrl field (e.g., add gatewayUrl: Schema.string()) and ensure ProviderStartOptions continues to reference OpenClawProviderStartOptions so providerOptions.openclaw.gatewayUrl is preserved by the contract.
🤖 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.
Outside diff comments:
In `@apps/web/src/components/ChatView.tsx`:
- Around line 4018-4044: The onIsAtEndChange handler never re-enables
tail-follow when the user manually scrolls back to the bottom; update the
handler so that when isAtEnd becomes true you restore tail-follow (e.g., call
the existing setter or update tailFollowEnabledRef.current = true — use
setTailFollowEnabled(true) if that setter exists) in addition to cancelling
debouncers and hiding the CTA; reference onIsAtEndChange and
tailFollowEnabledRef (and setTailFollowEnabled if present) to locate where to
add this restore logic.
- Around line 5668-5699: The code currently mutates nextThreadEnvMode to "local"
for first-message drafts (the block that sets nextThreadEnvMode = "local" and
calls setDraftThreadContext) before validating that a base branch is present,
which allows a missing branch to silently downgrade to local execution; revert
that change so validation runs first: remove or relocate the early downgrade and
setDraftThreadContext call from the start of the block and ensure the
shouldCreateWorktree check (using isFirstMessage, nextThreadEnvMode,
nextThreadWorktreePath, nextThreadBranch) runs while nextThreadEnvMode still
equals "worktree"; only set envMode to "local" and call setDraftThreadContext
(for isLocalDraftThread / threadIdForSend) after branch discovery is confirmed
to have failed and after the validation error has not been triggered (or
explicitly when you intend to convert the draft), and keep the
setStoreThreadError(threadIdForSend, "Select a base branch...") return path
intact.
In `@packages/contracts/src/providerDiscovery.ts`:
- Around line 109-119: OpenClawProviderStartOptions is currently an empty
Schema.Struct, so providerOptions.openclaw.gatewayUrl is dropped at decode
boundaries; update the OpenClawProviderStartOptions definition to include a
gatewayUrl field (e.g., add gatewayUrl: Schema.string()) and ensure
ProviderStartOptions continues to reference OpenClawProviderStartOptions so
providerOptions.openclaw.gatewayUrl is preserved by the contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0eca1ecb-5a17-42e9-a95d-8b5596daae36
📒 Files selected for processing (13)
apps/server/src/provider/runtimeLayer.tsapps/server/src/wsRpc.tsapps/web/src/components/ChatView.tsxapps/web/src/components/Sidebar.tsxapps/web/src/components/SkillLibrarySettingsPanel.tsxapps/web/src/composerDraftStore.test.tsapps/web/src/composerDraftStore.tsapps/web/src/wsNativeApi.tspackages/contracts/src/ipc.tspackages/contracts/src/providerDiscovery.tspackages/contracts/src/rpc.tspackages/contracts/src/ws.test.tspackages/contracts/src/ws.ts
- Fix challenge signing payload to use actual newline instead of literal backslash-n (critical HMAC bug) - Add openclaw to toLegacyProvider provider allowlist - Always use fixed gateway model for openclaw handoffs, ignoring sticky/project defaults - Return undefined for whitespace-only gateway URLs - Emit turn.completed for ack-only gateway responses - Include probe failure cause in health error messages - Restructure chat.send short-circuit to not miss gateway events with runId - Handle undefined from array indexing in loopback check - Update ADR index last-reviewed date - Use path.dirname in server settings test - Guard section markers in settings install test - Add regression test for non-gateway sticky selection with openclaw
There was a problem hiding this comment.
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 `@apps/web/src/routes/-_chat.settings.install.test.ts`:
- Around line 5-10: The current bounds check only verifies presence of markers
(defaultProviderStart and newThreadsStart) but not their order, which can yield
confusing slices if "New threads" precedes "Default provider"; update the logic
around defaultProviderStart, newThreadsStart and defaultProviderSectionSource to
assert or validate that defaultProviderStart < newThreadsStart before calling
settingsRouteSource.slice, and if the order is wrong produce a clear test
failure message (e.g., throw or assert with both marker positions and marker
texts) so tests fail fast with an informative error rather than producing
unexpected slice content.
🪄 Autofix (Beta)
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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef532430-cddb-4ce7-bc82-49e78a08a961
📒 Files selected for processing (12)
apps/server/src/provider/Layers/OpenClawAdapter.tsapps/server/src/provider/Layers/ProviderHealth.tsapps/server/src/provider/openclawGatewayClient.tsapps/server/src/provider/openclawGatewayProtocol.tsapps/server/src/provider/openclawGatewayUrl.tsapps/server/src/serverSettings.test.tsapps/web/src/lib/threadHandoff.test.tsapps/web/src/lib/threadHandoff.tsapps/web/src/routes/-_chat.settings.install.test.tsapps/web/src/store.tsdocs/adr/README.mdpackages/shared/src/serverSettings.ts
added 8 commits
June 5, 2026 22:46
- Fix TS error in threadHandoff regression test (double-cast invalid openclaw model fixtures to bypass discriminated union narrowing) - Re-enable tail-follow when user scrolls back to bottom in ChatView (pre-existing UX bug caught by reviewer) - Remove silent worktree-to-local downgrade before branch validation so the proper error message fires instead (pre-existing logic bug)
Effect's Result.Failure type does not expose a .cause property. The CodeRabbit suggestion to extract cause.message was incorrect. Simplified to a static error message matching the pattern used by all other probe failure branches in this file.
ProviderHealthLive (via LayerLive) requires ServerSettingsService but the test layer did not include it, causing Service not found errors for 11 of 17 tests. Adding the test layer double satisfies the dependency in the outer scope so the inner runtime layers resolve correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
New Features
Behavior / UX
Documentation