Skip to content

Conversation

@leavesster
Copy link
Contributor

  • refactor: use three componet
  • test: update test case

@coderabbitai
Copy link

coderabbitai bot commented Sep 26, 2025

Summary by CodeRabbit

  • 新功能
    • 凭证输入升级为“类型/名称/ID”三段式,校验更严格,错误提示更清晰。
  • Bug 修复
    • 阻止两段或空段凭证被接受,避免配置错误与潜在失败。
  • 测试
    • 补充三段式凭证与异常场景的单元测试覆盖。

Walkthrough

将凭证格式由两段改为三段。更新 CredentialInput 构造函数签名与内部表示;调整 generateCredentialInput 解析与校验逻辑以按逗号分割并验证三段;更新 replaceCredential 调用;相应地重写并扩展单元测试以覆盖三段格式与无效输入场景。

Changes

Cohort / File(s) Summary
Executor 源码:凭证模型与解析
packages/executor/src/credential.ts
CredentialInput 构造函数从两字段改为三字段 (type, name, id);generateCredentialInput 改为按逗号分割并校验三段非空;调用处改用新签名;replaceCredential 使用更新后的解析逻辑。
测试:凭证解析与替换
packages/executor/test/credential.test.ts
单元测试更新为三段格式(provider,key,token);新增/修改用例覆盖多段、少段、空段等无效输入;更新断言以匹配三字段对象与替换行为。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning 该标题未遵循“(): ”格式,既缺少冒号也未使用可选的作用域括号,因此无法明确反映主要变更内容。 请将标题更新为符合要求的格式,例如“refactor(executor): support three component credentials”,以便清晰表达此次重构目的。
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed 该描述虽较简要但与更改内容直接相关,明确指出了重构为三组件和更新测试用例的主要工作,符合描述检查要求。

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb2990 and c0eab9c.

📒 Files selected for processing (1)
  • packages/executor/src/credential.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/executor/src/credential.ts

Comment @coderabbitai help to get the list of available commands and usage tips.

@leavesster leavesster enabled auto-merge (squash) September 26, 2025 02:54
@leavesster leavesster disabled auto-merge September 26, 2025 02:54
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
packages/executor/src/credential.ts (1)

18-29: 健壮性:建议 trim 三段,兼容空白字符

当前带空格的片段会被原样接收,可能导致下游匹配失败;建议在拆分后 trim 再校验非空。

-  const parts = content.split(",");
+  const parts = content.split(",");
   if (parts.length !== 3) {
     return null;
   }
-
-  const [type, name, id] = parts;
-  if (!type || !name || !id) {
+  const [type, name, id] = parts.map((s) => s.trim());
+  if (!type || !name || !id) {
     return null;
   }
 
   return new CredentialInput(type, name, id);

另可移除魔法数字 17/-2,使用常量提升可读性(非本变更行,示意如下):

const PREFIX = "${{OO_CREDENTIAL:";
const SUFFIX = "}}";
if (!credentialPath.startsWith(PREFIX) || !credentialPath.endsWith(SUFFIX)) return null;
const content = credentialPath.slice(PREFIX.length, -SUFFIX.length).trim();
packages/executor/test/credential.test.ts (1)

29-33: 空片段用例覆盖到位;建议补充空白字符用例

当前已覆盖空字符串,建议新增包含空格的用例,配合实现层 trim 改进:

it("should trim spaces around parts", () => {
  expect(generateCredentialInput("${{OO_CREDENTIAL: github, token , my-token }}"))
    .toEqual(new CredentialInput("github", "token", "my-token"));
});
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8733a77 and 8cb2990.

📒 Files selected for processing (2)
  • packages/executor/src/credential.ts (2 hunks)
  • packages/executor/test/credential.test.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/executor/test/credential.test.ts (1)
packages/executor/src/credential.ts (3)
  • generateCredentialInput (8-29)
  • CredentialInput (4-6)
  • replaceCredential (31-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: check
🔇 Additional comments (10)
packages/executor/test/credential.test.ts (9)

7-10: 三段解析用例合理

覆盖标准三段路径解析,断言准确。


12-15: 特字符用例合理

验证连字符等常见字符,断言准确。


22-23: 两段输入判空逻辑覆盖到位

两段应返回 null,符合预期。


25-27: 超出三段的异常用例覆盖到位

多段输入返回 null,符合预期。


39-39: 替换输入的三段格式示例清晰

输入样例与 schema 搭配合理。


61-61: 替换结果断言正确

与新的三段构造保持一致。


67-67: 非 credential 类型不替换的前置条件设置合理

与 isCredentialSchema 语义一致。


81-82: 保持原值的断言正确

非 credential 类型保持字符串原样。


84-101: 无效格式不替换的用例完整

验证 replaceCredential 对无效路径的幂等性,符合预期。

packages/executor/src/credential.ts (1)

5-5: 确认:无两参调用及 .value 引用
已验证所有 new CredentialInput 均为三参,且未发现 .value 使用。

@leavesster leavesster merged commit ea8075f into main Sep 26, 2025
9 checks passed
@leavesster leavesster deleted the refactor-three-componet branch September 26, 2025 03:09
leavesster added a commit that referenced this pull request Sep 26, 2025
* refactor: use three componet

* test: update test case

* remove magic number
@oomol-bot oomol-bot mentioned this pull request Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants