feat(openclaw): 增加 OpenClaw 工具配置支持#11
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request expands the CLI helper to support Codex and OpenClaw integrations, enabling users to configure these assistants with Qiniu AI API endpoints. The changes include a new OpenClawTool implementation, registration of the tool in the manager, and updated documentation. Review feedback identifies a bug in base URL construction that could lead to redundant path segments, a logic error in the configuration status check, and a lack of error handling when executing external CLI commands. Additionally, a recommendation was made to refactor duplicated tool resolution logic into a shared utility.
|
|
||
| export function buildOpenClawProviderConfig(baseUrl: string, model: string): Record<string, unknown> { | ||
| return { | ||
| baseUrl: `${baseUrl.replace(/\/+$/, '')}/v1`, |
There was a problem hiding this comment.
Unconditionally appending /v1 to the baseUrl can lead to incorrect URLs if the provided baseUrl already includes the version suffix (e.g., https://api.qnaigc.com/v1 would become https://api.qnaigc.com/v1/v1). It's safer to check if the suffix is already present.
| baseUrl: `${baseUrl.replace(/\/+$/, '')}/v1`, | |
| baseUrl: baseUrl.replace(/\/+$/, '').endsWith('/v1') ? baseUrl.replace(/\/+$/, '') : `${baseUrl.replace(/\/+$/, '')}/v1`, |
| getConfig(): Record<string, unknown> { | ||
| return { | ||
| configPath: getOpenClawConfigPath(), | ||
| configured: this.getModelConfig()?.sonnetModel?.startsWith(`${PROVIDER_NAME}/`) || hasOpenClawConfigFile(), |
There was a problem hiding this comment.
The logic for the configured property is incorrect. this.getModelConfig() already slices off the ${PROVIDER_NAME}/ prefix, so calling startsWith on its result will always return false. Additionally, if getModelConfig() returns a valid configuration, it means the tool is already configured for Qiniu, making the redundant check unnecessary.
| configured: this.getModelConfig()?.sonnetModel?.startsWith(`${PROVIDER_NAME}/`) || hasOpenClawConfigFile(), | |
| configured: !!this.getModelConfig() || hasOpenClawConfigFile(), |
| function runOpenClawOperations(operations: OpenClawOperation[]): void { | ||
| for (const args of operations) { | ||
| execFileSync('openclaw', args, { stdio: 'pipe' }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The runOpenClawOperations function does not handle potential execution errors. If the openclaw CLI is not installed or if a command fails, execFileSync will throw an exception that could crash the application. Consider wrapping the execution in a try-catch block to provide a more user-friendly error message or to handle the failure gracefully.
| function runOpenClawOperations(operations: OpenClawOperation[]): void { | |
| for (const args of operations) { | |
| execFileSync('openclaw', args, { stdio: 'pipe' }); | |
| } | |
| } | |
| function runOpenClawOperations(operations: OpenClawOperation[]): void { | |
| try { | |
| for (const args of operations) { | |
| execFileSync('openclaw', args, { stdio: 'pipe' }); | |
| } | |
| } catch (error) { | |
| throw new Error(`Failed to execute OpenClaw command: ${error instanceof Error ? error.message : String(error)}`); | |
| } | |
| } |
| function resolveToolName(tool: string): string { | ||
| if (tool === 'claude' || tool === 'claude-code') return 'claude-code'; | ||
| if (tool === 'codex' || tool === 'openai-codex') return 'codex'; | ||
| if (tool === 'openclaw' || tool === 'claw') return 'openclaw'; | ||
| return tool; | ||
| } |
There was a problem hiding this comment.
The resolveToolName function is duplicated in both src/commands/auth.ts and src/lib/command.ts. This duplication makes the code harder to maintain. Consider moving this utility function to a shared location, such as src/lib/tool-manager.ts or a dedicated utility file.
1 participant
变更内容
设计边界
Fixes #10
验证