fix: address PR review feedback

- Remove CODE_REVIEW.md (internal development artifact) - Add bin field to package.json for CLI registration - Implement snapshot list command (was a stub) - Fix dead ternary in describeSecurityGroupAttribute - Remove unused metricName parameter from describeInstanceMonitorData Fixes issues identified by Greptile AI review.
2026-03-14 11:07:18 +08:00 · 2026-03-14 11:07:18 +08:00 · 3aeb2134fa
commit 3aeb2134fa
parent d17105ef7d
4 changed files with 21 additions and 266 deletions
--- a/skills/aliyun-ecs-skill/CODE_REVIEW.md
+++ b/skills/aliyun-ecs-skill/CODE_REVIEW.md
@ -1,252 +0,0 @@
-# 阿里云ECS Skill 代码审查报告
-
-**审查时间**: 2026-03-12  
-**审查人**: AI Agent  
-**代码版本**: v1.0.0 (开发版)
-
---
-
-## 一、总体评估
-
-| 项目 | 状态 | 说明 |
-|------|------|------|
-| **语法正确性** | ✅ 通过 | 所有JS文件语法检查通过 |
-| **代码结构** | ✅ 良好 | 模块化设计，职责分离 |
-| **代码量** | ⚠️ 适中 | 693行（ecs.js: 337行, index.js: 356行）|
-| **注释覆盖** | ⚠️ 不足 | 需要补充关键函数注释 |
-| **错误处理** | ⚠️ 需完善 | 部分场景缺少错误捕获 |
-
---
-
-## 二、详细检查结果
-
-### 2.1 文件结构 ✅
-
-```
-aliyun-ecs-skill/
-├── SKILL.md              ✅ 完整（使用文档）
-├── README.md             ✅ 完整（项目说明）
-├── package.json          ✅ 语法正确
-├── _meta.json            ✅ 简单配置
-├── scripts/
-│   └── setup.sh          ✅ 可执行，逻辑清晰
-└── src/
-    ├── index.js          ✅ CLI入口（356行）
-    └── api/
-        └── ecs.js        ✅ API封装（337行）
-```
-
-**评价**: 目录结构符合OpenClaw Skill规范
-
---
-
-### 2.2 package.json ✅
-
-```json
-{
-  "name": "aliyun-ecs-skill",
-  "version": "1.0.0",
-  "description": "Aliyun ECS management skill for OpenClaw",
-  "main": "src/index.js",
-  "dependencies": {
-    "@alicloud/openapi-client": "^0.4.10",
-    "@alicloud/ecs20140526": "^7.0.0"
-  }
-}
-```
-
-**检查项**:
- ✅ 名称符合规范
- ✅ 依赖版本明确
- ✅ 入口文件正确
- ⚠️ 建议添加: `"bin": { "aliyun-ecs": "./src/index.js" }`
-
---
-
-### 2.3 API封装 (ecs.js) ✅
-
-**已实现功能**:
-1. ✅ describeRegions() - 查询地域列表
-2. ✅ describeInstances() - 查询实例列表
-3. ✅ startInstance() - 启动实例
-4. ✅ stopInstance() - 停止实例
-5. ✅ rebootInstance() - 重启实例
-6. ✅ describeInstanceMonitorData() - 监控数据
-7. ✅ createSnapshot() - 创建快照
-8. ✅ describeSnapshots() - 查询快照
-9. ✅ resetDisk() - 回滚快照
-10. ✅ describeSecurityGroups() - 查询安全组
-11. ✅ describeSecurityGroupAttribute() - 查询安全组规则
-12. ✅ authorizeSecurityGroup() - 添加安全组规则
-13. ✅ revokeSecurityGroup() - 删除安全组规则
-
-**潜在问题**:
- ⚠️ 第38-42行: 配置加载失败时没有友好提示
- ⚠️ 第201行: `resetDisk` 实际应该是 `resetDisk` API，但阿里云ECS回滚快照是用 `ResetDisk` 接口，需要确认参数
- ⚠️ 缺少分页处理（当实例很多时）
-
-**建议改进**:
-```javascript
-// 添加超时处理
-const client = new Ecs20140526(clientConfig, { timeout: 30000 });
-
-// 添加重试逻辑
-async function withRetry(fn, retries = 3) {
-  for (let i = 0; i < retries; i++) {
-    try {
-      return await fn();
-    } catch (err) {
-      if (i === retries - 1) throw err;
-      await new Promise(r => setTimeout(r, 1000 * (i + 1)));
-    }
-  }
-}
-```
-
---
-
-### 2.4 CLI工具 (index.js) ✅
-
-**命令覆盖**:
- ✅ regions
- ✅ list
- ✅ info
- ✅ start
- ✅ stop
- ✅ restart
- ⚠️ monitor（只有框架，未完全实现）
- ✅ snapshot list/create/rollback
- ✅ security-group list/rules/add/remove
-
-**代码问题**:
- ⚠️ 第45-53行: 参数解析逻辑有缺陷，无法正确处理 `--flag` 形式的布尔参数
- ⚠️ 第178-183行: monitor命令未完成实现
- ⚠️ 缺少 `--help` 全局帮助
-
-**建议修复**:
-```javascript
-// 改进参数解析
-const args = process.argv.slice(2);
-const command = args[0];
-const options = {};
-
-for (let i = 1; i < args.length; i++) {
-  if (args[i].startsWith('--')) {
-    const key = args[i].slice(2);
-    if (i + 1 < args.length && !args[i + 1].startsWith('--')) {
-      options[key] = args[i + 1];
-      i++;
-    } else {
-      options[key] = true;
-    }
-  }
-}
-```
-
---
-
-### 2.5 setup.sh 脚本 ✅
-
-**功能检查**:
- ✅ Node.js版本检查
- ✅ SDK安装
- ✅ 配置文件创建
- ✅ 权限设置（chmod 600）
- ⚠️ 测试连接功能未完成（第129-142行是占位符）
-
-**建议完成测试连接**:
-```bash
-# 实际调用阿里云API测试
-test_connection() {
-    echo "正在验证阿里云连接..."
-    
-    local result=$(curl -s "https://ecs.aliyuncs.com/?Action=DescribeRegions&Format=JSON&Version=2014-05-26&AccessKeyId=$(jq -r .accessKeyId $CONFIG_FILE)&SignatureMethod=HMAC-SHA1&Timestamp=$(date -u +%Y-%m-%dT%H:%M:%SZ)&SignatureVersion=1.0" 2>&1)
-    
-    if echo "$result" | grep -q "Region"; then
-        echo -e "${GREEN}✓ 连接验证成功${NC}"
-        return 0
-    else
-        echo -e "${RED}✗ 连接验证失败，请检查密钥${NC}"
-        return 1
-    fi
-}
-```
-
---
-
-### 2.6 文档检查
-
-**SKILL.md** ✅:
- 格式符合OpenClaw规范
- 功能说明完整
- 使用示例充分
-
-**README.md** ✅:
- 项目介绍清晰
- 安装步骤详细
- 使用场景丰富
-
---
-
-## 三、关键问题汇总
-
-| 优先级 | 问题 | 文件 | 影响 | 建议 |
-|--------|------|------|------|------|
-| 🔴 P0 | 连接测试未完成 | setup.sh | 用户无法验证配置 | 补充API调用测试 |
-| 🔴 P0 | monitor命令未实现 | index.js | 功能缺失 | 完成监控数据查询 |
-| 🟡 P1 | 参数解析缺陷 | index.js | 某些参数解析错误 | 改进解析逻辑 |
-| 🟡 P1 | 缺少分页 | ecs.js | 实例多时无法显示 | 添加分页参数 |
-| 🟢 P2 | 缺少注释 | ecs.js | 可读性降低 | 补充JSDoc注释 |
-| 🟢 P2 | 缺少超时重试 | ecs.js | 网络问题可能失败 | 添加重试逻辑 |
-
---
-
-## 四、优化建议
-
-### 4.1 立即修复（测试前）
-
-1. **完成monitor命令实现** (index.js 178-183行)
-2. **修复参数解析** (index.js 45-53行)
-3. **完成setup.sh测试连接功能** (129-142行)
-
-### 4.2 测试期间改进
-
-4. **添加错误日志** - 便于调试
-5. **补充JSDoc注释** - 提高可维护性
-6. **添加输入验证** - 防止非法参数
-
-### 4.3 PR前优化
-
-7. **添加单元测试** - 至少测试核心函数
-8. **添加CHANGELOG.md** - 版本记录
-9. **优化README截图** - 添加使用效果截图
-
---
-
-## 五、预计修复时间
-
-| 任务 | 时间 |
-|------|------|
-| 修复关键问题（P0） | 1-2小时 |
-| 完成优化建议（P1-P2） | 2-3小时 |
-| **总计** | **3-5小时** |
-
---
-
-## 六、结论
-
-**当前状态**: 代码基本可用，但有关键功能未完成
-
-**建议**:
-1. ⚠️ **测试前必须修复**: monitor命令和setup.sh测试连接
-2. ✅ **整体质量**: 代码结构良好，符合OpenClaw规范
-3. ✅ **功能覆盖**: 核心功能（实例/快照/安全组）已实现
-
-**预计明天测试时的问题**:
- 如果遇到连接问题，检查setup.sh的测试连接输出
- monitor命令会提示"需要进一步开发"，这是正常的
-
---
-
-**审查完成时间**: 2026-03-12 03:35  
-**建议行动**: 先修复P0问题，再进行测试
--- a/skills/aliyun-ecs-skill/package.json
+++ b/skills/aliyun-ecs-skill/package.json
@ -3,6 +3,9 @@
  "version": "1.0.0",
  "description": "Aliyun ECS management skill for OpenClaw",
  "main": "src/index.js",
+  "bin": {
+    "aliyun-ecs": "./src/index.js"
+  },
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
--- a/skills/aliyun-ecs-skill/src/api/ecs.js
+++ b/skills/aliyun-ecs-skill/src/api/ecs.js
@ -133,7 +133,7 @@ async function rebootInstance(regionId, instanceId, forceStop = false) {
 }

 // 获取监控数据
-async function describeInstanceMonitorData(regionId, instanceId, metricName, period = 60, startTime, endTime) {
+async function describeInstanceMonitorData(regionId, instanceId, period = 60, startTime, endTime) {
    const client = createClient(regionId);
    
    const request = new Ecs20140526.DescribeInstanceMonitorDataRequest({
@ -264,9 +264,7 @@ async function describeSecurityGroupAttribute(regionId, securityGroupId, directi
    
    const response = await client.describeSecurityGroupAttribute(request);
    
-    const rules = direction === 'ingress' 
-        ? response.body.permissions?.permission 
-        : response.body.permissions?.permission;
+    const rules = response.body.permissions?.permission;
    
    if (!rules) return [];
    
--- a/skills/aliyun-ecs-skill/src/index.js
+++ b/skills/aliyun-ecs-skill/src/index.js
@ -242,7 +242,6 @@ async function main() {
                const monitorData = await ecs.describeInstanceMonitorData(
                    options.region,
                    options.id,
-                    null,
                    period,
                    startTime.toISOString(),
                    endTime.toISOString()
@ -284,16 +283,23 @@ async function main() {
                        console.error('错误: 请提供 --region 和 --id 参数');
                        process.exit(1);
                    }
-                    // 查询实例的磁盘ID，然后查询快照
-                    const instances = await ecs.describeInstances(options.region, {
-                        instanceIds: JSON.stringify([options.id]),
-                    });
-                    if (instances.length === 0) {
-                        console.log('实例不存在');
-                        process.exit(1);
+                    // 查询实例的快照列表
+                    const snapshots = await ecs.describeSnapshots(options.region, options.id, null);
+                    console.log(`\n实例 ${options.id} 的快照列表:\n`);
+                    if (snapshots.length === 0) {
+                        console.log('暂无快照');
+                    } else {
+                        console.log(formatTable(snapshots.map(snap => ({
+                            ...snap,
+                            status: snap.status === 'accomplished' ? '已完成' : snap.status,
+                        })), [
+                            { key: 'snapshotId', header: '快照ID' },
+                            { key: 'snapshotName', header: '名称' },
+                            { key: 'status', header: '状态' },
+                            { key: 'progress', header: '进度' },
+                            { key: 'creationTime', header: '创建时间' },
+                        ]));
                    }
-                    // 注意：需要通过其他API获取磁盘ID，这里简化处理
-                    console.log('快照列表功能需要进一步开发');
                } else if (subCommand === 'create') {
                    if (!options.region || !options['disk-id'] || !options.name) {
                        console.error('错误: 请提供 --region, --disk-id 和 --name 参数');