feat: add RuntimeAdapter interface and OpenClawGatewayAdapter (Phase 1)#499
feat: add RuntimeAdapter interface and OpenClawGatewayAdapter (Phase 1)#499HiddenPuppy wants to merge 3 commits into
Conversation
Implements Phase 1 of the ClawWork Next RuntimeAdapter architecture. - Adds shared RuntimeAdapter interface and normalized ExecutionEvent types - Creates RuntimeAdapterPort for dependency injection - Implements OpenClawGatewayAdapter wrapping GatewayTransportPort - Bridges OpenClaw Gateway events (chat/agent/approval) to unified events - No user-visible behavior change; backward compatible Fixes #498
|
Hi @HiddenPuppy, DetailsInstructions for interacting with me using comments are available here. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements the first phase of the ClawWork Next RuntimeAdapter architecture. By decoupling the core services from specific transport implementations, it establishes a standardized interface for interacting with various agent runtimes. The changes provide a unified event system and abstraction layer, ensuring that future runtime integrations can be added without modifying core business logic. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new RuntimeAdapter abstraction layer to decouple the core service logic from specific agent runtimes like OpenClaw Gateway or ACP. It includes the definition of the RuntimeAdapter interface and capabilities in the shared package, a dependency injection port in the core package, and a concrete implementation for the OpenClaw Gateway. Feedback focuses on critical reliability and logic issues: unsafe JSON parsing of tool arguments could crash the event listener, a bug in the unsubscription logic prevents re-subscribing to gateway events, and inconsistent event mapping for approval resolutions may break UI correlation.
| toolCallId: toolData.toolCallId, | ||
| name: toolData.name, | ||
| status: status as 'running' | 'done' | 'error', | ||
| args: toolData.args ? JSON.parse(toolData.args) : undefined, |
There was a problem hiding this comment.
Using JSON.parse directly on data received from the gateway is unsafe. If the gateway sends malformed JSON or an empty string in the args field, this will throw an exception and crash the entire event listener for this adapter. Consider using a safe JSON parsing utility or wrapping this in a try-catch block.
| args: toolData.args ? JSON.parse(toolData.args) : undefined, | |
| args: toolData.args ? (() => { try { return JSON.parse(toolData.args); } catch { return undefined; } })() : undefined, |
| if (listeners.size === 0) { | ||
| removeGatewayEvent(); | ||
| removeGatewayStatus(); | ||
| } |
There was a problem hiding this comment.
This cleanup logic introduces a bug. Once removeGatewayEvent() and removeGatewayStatus() are called (when the last listener unsubscribes), the transport listeners are permanently detached for this adapter instance. If a new listener is added later via onExecutionEvent, it will never receive events because the transport connection is not re-established.
Since this adapter is intended to be a long-lived service wrapping the transport, it is safer to remove this premature cleanup or implement a reference-counting mechanism that re-subscribes when the first listener is added.
| if (listeners.size === 0) { | |
| removeGatewayEvent(); | |
| removeGatewayStatus(); | |
| } | |
| if (listeners.size === 0) { | |
| // Avoid permanent unsubscription here as it prevents re-subscription later. | |
| // If cleanup is required, implement ref-counting to re-establish listeners. | |
| } |
| const payload = data.payload as { id: string; decision?: string }; | ||
| emit({ | ||
| type: 'execution.approval.resolved', | ||
| executionId: data.gatewayId, | ||
| runtimeId: data.gatewayId, | ||
| timestamp: Date.now(), | ||
| id: payload.id, | ||
| decision: payload.decision, | ||
| }); |
There was a problem hiding this comment.
The execution.approval.resolved event is missing the sessionKey and uses data.gatewayId as the executionId. This is inconsistent with the execution.approval.requested event (lines 161-172) and will likely break correlation in the UI if the approval was associated with a specific session. You should extract the sessionKey from the payload if available.
const payload = data.payload as { id: string; decision?: string };
const sessionKey = (data.payload as Record<string, unknown>).sessionKey as string | undefined;
emit({
type: 'execution.approval.resolved',
executionId: sessionKey || data.gatewayId,
runtimeId: data.gatewayId,
sessionKey,
timestamp: Date.now(),
id: payload.id,
decision: payload.decision,
});
Summary
Implements Phase 1 of the ClawWork Next RuntimeAdapter architecture (issue #498). Creates a runtime-agnostic
RuntimeAdapterinterface and wraps the existing OpenClaw Gateway transport behind it withOpenClawGatewayAdapter.Changes
RuntimeAdapterinterface,RuntimeCapabilities,ExecutionRef, normalizedExecutionEventtypes, andExecutionRegistryGatewayTransportPortbehindRuntimeAdapter, bridging OpenClaw Gateway events (chat/agent/approval) into unifiedExecutionEventformatTesting
Existing test suite passes. No user-visible behavior change — the existing GatewayTransportPort-based flow remains the default direct mode.
Fixes #498