feat(providers): add OpenClaw multi-provider support #7562
Conversation
Add four OpenClaw provider types for evaluating agents via the OpenClaw gateway: Chat Completions (OpenAI-compatible), Responses API, WebSocket Agent (native RPC protocol), and Tool Invoke (HTTP). Includes auto-detection of gateway URL and auth token from ~/.openclaw/openclaw.json, env vars, or explicit config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ases Prevent OpenAI env var fallback (OPENAI_API_KEY, OPENAI_BASE_URL) in chat/responses providers, fix wildcard bind detection (0.0.0.0/::), fix concurrent WS socket leak, scope thinking_level to WS Agent only, and harden JSON5 trailing comma parser against string contents. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove duplicate env vars example section (already covered in Configuration), add policy-gated 404 note to Tool Invoke, and dedupe Claude Agent SDK row in providers index table. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
⏩ No test execution environment matched (3cf9df9) View output ↗ View check history
|
mldangelo
requested review from
a team,
MrFlounder,
danenania,
faizanminhas and
typpo
as code owners
![promptfoo-scanner[bot]](https://avatars.githubusercontent.com/in/2172945?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 All Clear
I reviewed this PR which adds OpenClaw provider integration to promptfoo. The code implements client adapters that forward requests to an external OpenClaw gateway service. I examined authentication handling, configuration parsing, tool invocation proxying, and data flows. No LLM security vulnerabilities were found.
Minimum severity threshold: 🟡 Medium | To re-scan after changes, comment @promptfoo-scanner
Learn more
Add tests for IPv6 wildcard bind, env override params, JSON5 edge cases (single quotes, block comments in strings, commas in strings), apiKeyRequired flag, custom header merging, toJSON serialization, HTTPS→WSS conversion, malformed frame handling, non-assistant stream filtering, tool name with colons, and resetConfigCache behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔄 Security Review ✅No issues in new changes. The incremental commit ( Last updated: 2026-02-09T09:00:00Z | Reviewing: 3cf9df9 |
There was a problem hiding this comment.
👍 All Clear
I reviewed this PR for LLM-specific security vulnerabilities. The PR adds OpenClaw provider integration following standard SDK patterns for credential handling and API communication. No exploitable LLM security vulnerabilities were found.
Minimum severity threshold: 🟡 Medium | To re-scan after changes, comment @promptfoo-scanner
Learn more
📝 WalkthroughWalkthroughThis PR adds comprehensive support for the OpenClaw agent framework as a new provider type in promptfoo. The implementation includes four provider variants (Chat, Responses, WebSocket Agent, Tool Invoke), shared configuration utilities for gateway URL and authentication token resolution, environment variable definitions, type definitions, a factory pattern for provider instantiation, provider registry integration, example configuration and documentation, and an extensive test suite covering all provider types and utility functions. All changes are additive with no modifications to existing code. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/providers/openclaw/shared.ts`:
- Around line 52-58: The block-comment skipping logic can advance past the end
when the comment is unclosed; update the handler around the variables ch, raw
and index i so that after the inner while you only do the i += 2 advance if you
actually found the closing "*/" (i.e., raw[i] === '*' && raw[i+1] === '/');
otherwise set i = raw.length (or break/return from the containing function) to
stop parsing and avoid an out-of-bounds advance. Locate this logic in the block
that tests ch === '/' && raw[i + 1] === '*' and apply the conditional advance to
ensure i is not pushed beyond raw.length.
🧹 Nitpick comments (6)
site/docs/providers/openclaw.md (1)
1-6: Front matter description is shorter than recommended.The description is approximately 104 characters, which is below the recommended 150-160 character range per coding guidelines. Consider expanding it slightly for better SEO and clarity.
--- title: OpenClaw sidebar_label: OpenClaw sidebar_position: 42 -description: 'Use OpenClaw, a personal AI assistant framework, as an eval target with auto-detected gateway and auth' +description: 'Use OpenClaw, a personal AI assistant framework, as an eval target with auto-detected gateway URL and authentication token from config or environment' ---As per coding guidelines: "Front matter is required with title (under 60 chars), description (150-160 chars), and sidebar_position"
src/providers/openclaw/index.ts (1)
33-63: Minor: Consider mergingenvmore carefully.Line 40 overwrites any existing
envinproviderOptionswith theenvparameter. If a caller provides bothproviderOptions.envand theenvparameter, the latter completely replaces the former rather than merging them.This might be intentional (env parameter takes precedence), but consider if merging would be more appropriate:
- const opts = { ...providerOptions, env }; + const opts = { ...providerOptions, env: { ...providerOptions.env, ...env } };If the current behavior is intentional (full override), consider adding a brief comment for clarity.
test/providers/openclaw.test.ts (3)
407-559: Consider adding rate limit and token usage tracking tests forOpenClawToolInvokeProvider.Per coding guidelines, provider tests should cover rate limits (429 responses) and token usage tracking. The current tests cover success, tool errors, HTTP errors (404), auth tokens, and network errors, but are missing:
- Rate limit handling (HTTP 429)
- Token usage tracking in responses
💡 Suggested additional test cases
it('should handle rate limit errors (429)', async () => { const provider = new OpenClawToolInvokeProvider('bash', { config: { gateway_url: 'http://test:18789' }, }); mockFetchWithProxy.mockResolvedValue({ ok: false, status: 429, text: async () => 'Too Many Requests', } as Response); const result = await provider.callApi('{}'); expect(result.error).toContain('429'); });Based on learnings: "Every provider must have tests covering: success cases, error cases (4xx, 5xx, rate limits), configuration validation, and token usage tracking"
561-577: Consider using a more specific type formessageHandlers.The
Functiontype is overly broad. Using a more specific callback signature improves type safety.🔧 Suggested type improvement
describe('OpenClawAgentProvider', () => { let mockWs: any; - let messageHandlers: Map<string, Function>; + let messageHandlers: Map<string, (data: Buffer | Error) => void>;
974-983: Good cleanup test, but consider verifyingactiveConnectionsis cleared.The test verifies
ws.close()is called but doesn't verify the internalactiveConnectionsset is cleared. Sincecleanup()explicitly clears this set, consider adding an assertion or a second call to ensure idempotency.💡 Suggested enhancement
it('should cleanup WS connection', async () => { const provider = new OpenClawAgentProvider('main', { config: { gateway_url: 'http://test:18789' }, }); // Start a call to create a WS provider.callApi('Hello'); await provider.cleanup(); expect(mockWs.close).toHaveBeenCalled(); + + // Verify cleanup is idempotent + mockWs.close.mockClear(); + await provider.cleanup(); + expect(mockWs.close).not.toHaveBeenCalled(); });src/providers/openclaw/shared.ts (1)
120-122: Use structured logging with object context.Per coding guidelines, logger calls should use object context for auto-sanitization rather than string interpolation.
🔧 Suggested fix
} catch (err) { - logger.debug(`Failed to read OpenClaw config at ${configPath}: ${err}`); + logger.debug('[OpenClaw] Failed to read config', { configPath, error: err }); return undefined; }As per coding guidelines: "Use the logger with object context (auto-sanitized) for logging in format:
logger.debug('[Component] Message', { headers, body, config })"
src/providers/openclaw/shared.ts
Outdated
| } else if (ch === '/' && raw[i + 1] === '*') { | ||
| // Block comment — skip to */ | ||
| i += 2; | ||
| while (i < raw.length - 1 && !(raw[i] === '*' && raw[i + 1] === '/')) { | ||
| i++; | ||
| } | ||
| i += 2; // skip */ |
There was a problem hiding this comment.
Potential out-of-bounds read in block comment handling.
If the input ends with an unclosed block comment (e.g., /* no end), the inner while loop condition i < raw.length - 1 prevents reading past the end, but the subsequent i += 2 at line 58 could push i beyond raw.length. This doesn't cause a runtime error (just iterates with undefined characters), but the logic could be tightened.
🛡️ Suggested defensive fix
} else if (ch === '/' && raw[i + 1] === '*') {
// Block comment — skip to */
i += 2;
while (i < raw.length - 1 && !(raw[i] === '*' && raw[i + 1] === '/')) {
i++;
}
- i += 2; // skip */
+ // Skip closing */ if found
+ if (i < raw.length - 1 && raw[i] === '*' && raw[i + 1] === '/') {
+ i += 2;
+ }🤖 Prompt for AI Agents
In `@src/providers/openclaw/shared.ts` around lines 52 - 58, The block-comment
skipping logic can advance past the end when the comment is unclosed; update the
handler around the variables ch, raw and index i so that after the inner while
you only do the i += 2 advance if you actually found the closing "*/" (i.e.,
raw[i] === '*' && raw[i+1] === '/'); otherwise set i = raw.length (or
break/return from the containing function) to stop parsing and avoid an
out-of-bounds advance. Locate this logic in the block that tests ch === '/' &&
raw[i + 1] === '*' and apply the conditional advance to ensure i is not pushed
beyond raw.length.
Add bounds check before advancing past block comment close token to prevent out-of-bounds read when input ends with an unclosed comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rror The WebSocketMock vi.fn() had no parameters, causing TypeScript to infer mock.calls[0] as an empty tuple. This broke `tsc --noEmit` in CI, which SIGTERMed the rest of the build. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
openclaw:main), Responses (openclaw:responses:main), WebSocket Agent (openclaw:agent:main), and Tool Invoke (openclaw:tools:bash)~/.openclaw/openclaw.jsonwith env var and explicit config overridesOPENAI_API_KEY,OPENAI_BASE_URL) from leaking into OpenClaw providersTest plan
🤖 Generated with Claude Code