feat: add OpenClaw workflow node#1535
Conversation
|
|
There was a problem hiding this comment.
Code Review
This pull request introduces the OpenClaw/ChatClaw workflow node template, integrating it into both the frontend workflow builder and the backend execution engine. The backend implementation extends the existing MCP node to support OpenClaw skill execution, accompanied by unit tests. The frontend changes add the configuration UI, registration, and validation logic for the new node type. The review feedback highlights potential runtime TypeError crashes in the frontend if nodeParam is undefined or null during initialization or validation, suggesting safe guards for helper functions and parameter validation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const readString = ( | ||
| nodeParam: Record<string, unknown>, | ||
| key: keyof OpenClawNodeParam | ||
| ): string => { | ||
| const value = nodeParam[key]; | ||
| return typeof value === 'string' ? value : ''; | ||
| }; | ||
|
|
||
| const readTuningParams = ( | ||
| nodeParam: Record<string, unknown> | ||
| ): OpenClawTuningParams => { | ||
| const value = nodeParam.tuningParams; | ||
| if (!value || typeof value !== 'object' || Array.isArray(value)) { | ||
| return {}; | ||
| } | ||
| return value as OpenClawTuningParams; | ||
| }; |
There was a problem hiding this comment.
If nodeParam is undefined or null (which can happen during initialization or if the node state is empty/corrupted), calling readString or readTuningParams will throw a TypeError and crash the entire React render tree.
We should update these helper functions to safely handle a potentially missing nodeParam object.
const readString = (
nodeParam: Record<string, unknown> | undefined | null,
key: keyof OpenClawNodeParam
): string => {
if (!nodeParam) return '';
const value = nodeParam[key];
return typeof value === 'string' ? value : '';
};
const readTuningParams = (
nodeParam: Record<string, unknown> | undefined | null
): OpenClawTuningParams => {
if (!nodeParam) return {};
const value = nodeParam.tuningParams;
if (!value || typeof value !== 'object' || Array.isArray(value)) {
return {};
}
return value as OpenClawTuningParams;
};
There was a problem hiding this comment.
Fixed in 842cdc4: readString and readTuningParams now accept null/undefined node params and return safe defaults before reading fields.
| function validateOpenClawParams(currentCheckNode: unknown): boolean { | ||
| if (currentCheckNode?.nodeType !== 'openclaw') { | ||
| return true; | ||
| } | ||
|
|
||
| const nodeParam = currentCheckNode.data.nodeParam; | ||
| const valueCannotBeEmpty = i18next.t( | ||
| 'workflow.nodes.validation.valueCannotBeEmpty' | ||
| ); | ||
| let passFlag = true; | ||
|
|
||
| if (!nodeParam?.mcpServerId?.trim() && !nodeParam?.mcpServerUrl?.trim()) { | ||
| nodeParam.mcpServerUrlErrMsg = valueCannotBeEmpty; | ||
| passFlag = false; | ||
| } else if ( | ||
| nodeParam?.mcpServerUrl?.trim() && | ||
| !isValidURL(nodeParam.mcpServerUrl) | ||
| ) { | ||
| nodeParam.mcpServerUrlErrMsg = i18next.t( | ||
| 'workflow.nodes.validation.pleaseEnterValidURL' | ||
| ); | ||
| passFlag = false; | ||
| } else { | ||
| nodeParam.mcpServerUrlErrMsg = ''; | ||
| } | ||
|
|
||
| if (!nodeParam?.toolName?.trim()) { | ||
| nodeParam.toolNameErrMsg = valueCannotBeEmpty; | ||
| passFlag = false; | ||
| } else { | ||
| nodeParam.toolNameErrMsg = ''; | ||
| } | ||
|
|
||
| if (!nodeParam?.skillName?.trim()) { | ||
| nodeParam.skillNameErrMsg = valueCannotBeEmpty; | ||
| passFlag = false; | ||
| } else { | ||
| nodeParam.skillNameErrMsg = ''; | ||
| } | ||
|
|
||
| if (!nodeParam?.executionMode?.trim()) { | ||
| nodeParam.executionModeErrMsg = valueCannotBeEmpty; | ||
| passFlag = false; | ||
| } else { | ||
| nodeParam.executionModeErrMsg = ''; | ||
| } | ||
|
|
||
| return passFlag; | ||
| } |
There was a problem hiding this comment.
If nodeParam is undefined or null, attempting to assign error messages (e.g., nodeParam.mcpServerUrlErrMsg = ...) will throw a runtime TypeError and crash the validation process.
We should safely guard against a missing nodeParam object.
function validateOpenClawParams(currentCheckNode: unknown): boolean {
if ((currentCheckNode as any)?.nodeType !== 'openclaw') {
return true;
}
const nodeParam = (currentCheckNode as any)?.data?.nodeParam;
if (!nodeParam) {
return false;
}
const valueCannotBeEmpty = i18next.t(
'workflow.nodes.validation.valueCannotBeEmpty'
);
let passFlag = true;
if (!nodeParam.mcpServerId?.trim() && !nodeParam.mcpServerUrl?.trim()) {
nodeParam.mcpServerUrlErrMsg = valueCannotBeEmpty;
passFlag = false;
} else if (
nodeParam.mcpServerUrl?.trim() &&
!isValidURL(nodeParam.mcpServerUrl)
) {
nodeParam.mcpServerUrlErrMsg = i18next.t(
'workflow.nodes.validation.pleaseEnterValidURL'
);
passFlag = false;
} else {
nodeParam.mcpServerUrlErrMsg = '';
}
if (!nodeParam.toolName?.trim()) {
nodeParam.toolNameErrMsg = valueCannotBeEmpty;
passFlag = false;
} else {
nodeParam.toolNameErrMsg = '';
}
if (!nodeParam.skillName?.trim()) {
nodeParam.skillNameErrMsg = valueCannotBeEmpty;
passFlag = false;
} else {
nodeParam.skillNameErrMsg = '';
}
if (!nodeParam.executionMode?.trim()) {
nodeParam.executionModeErrMsg = valueCannotBeEmpty;
passFlag = false;
} else {
nodeParam.executionModeErrMsg = '';
}
return passFlag;
}There was a problem hiding this comment.
Fixed in 842cdc4: validateOpenClawParams now guards missing data.nodeParam before assigning error fields, so validation fails safely instead of throwing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ab277f6c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ['plugin', 'flow', 'openclaw'].includes(currentCheckNode?.nodeType) | ||
| ? currentCheckNode?.data?.inputs | ||
| ?.filter(input => !input?.required || input?.disabled) | ||
| ?.map(input => input?.id) |
There was a problem hiding this comment.
Don’t skip blank OpenClaw inputs that runtime still reads
For OpenClaw nodes this treats every non-required input as safe to leave unresolved, including the default optional context input that is saved as a ref with empty content. The runtime OpenClawNode inherits MCPNode.execute, which iterates every input_identifier and calls variable_pool.get_variable for it, so a workflow that only fills the required instruction passes canvas validation but fails before the MCP call when context resolves to an empty reference. Either initialize optional OpenClaw inputs as literals or have the OpenClaw runtime omit unresolved optional args.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 842cdc4: MCPNode now resolves inputs through an overridable collect_inputs hook, and OpenClawNode skips unresolved or empty configured optional inputs such as context. Added regression coverage in test_openclaw_skips_unresolved_optional_input.
4ab277f to
a10dc65
Compare
a10dc65 to
842cdc4
Compare
|
Could you take a look at the CI issues? Thanks! |
FenjuFu
left a comment
There was a problem hiding this comment.
The feature direction fits the #1066 reward scope, but this isn't reviewable for merge yet:
- DCO check fails — commits need a
Signed-off-byline (git rebase --signoffthen force-push). - CLA is unsigned — see the cla-assistant link in the checks.
- console-frontend lint job is red.
Please get those green and I'll do a full review.
Closes #1066.
Summary
openclawworkflow node backed by the existing MCP gateway request shape.nodeParamvalues and skipping unresolved optional OpenClaw inputs such ascontextat runtime.Validation
UV_CACHE_DIR=C:\Users\maxjiang\Documents\YY\external\astron-agent\.uv-cache uv run --python 3.11 pytest tests\engine\nodes\test_openclaw_node.py(5 passed, warnings only)UV_CACHE_DIR=C:\Users\maxjiang\Documents\YY\external\astron-agent\.uv-cache uv run --python 3.11 black --check engine\nodes\mcp\mcp_node.py engine\nodes\openclaw\openclaw_node.py tests\engine\nodes\test_openclaw_node.pygit diff --checkgit diff --cached --checknpm.cmd run type-checkcould not run in this checkout becauseconsole/frontend/node_modulesis missing andtscis not available.Notes
action_requiredbecause CLA signing is still pending.