refactor(layer): use structured BindPath type instead of string[]#421
refactor(layer): use structured BindPath type instead of string[]#421leavesster merged 1 commit intomainfrom
Conversation
Align createPackageLayer with the BindPath type already defined in oocana.ts, removing redundant string validation regex and using the same conversion logic as buildRunConfigArgs.
Summary by CodeRabbit发布说明
✏️ Tip: You can customize this high-level summary in your review settings. 总体概览此拉取请求对绑定路径配置API进行了重构,将基于字符串的参数 变更详情
估计代码审查工作量🎯 2 (Simple) | ⏱️ ~10 分钟 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the createPackageLayer function to use a structured BindPath type instead of string arrays for bind path configuration, improving type safety and consistency with the existing buildRunConfigArgs function.
Changes:
- Replaced
bind_paths?: string[]parameter withbindPaths?: BindPath[]increatePackageLayer - Removed redundant
bindPathPatternregex validation and non-functional error message - Implemented the same structured-to-string conversion logic used in
buildRunConfigArgs
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/oocana/src/layer.ts | Updated createPackageLayer to accept BindPath[] type and use structured conversion logic matching buildRunConfigArgs |
| flow-examples/test/layer.test.ts | Updated test to use the new structured BindPath object format instead of string format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/oocana/src/layer.ts`:
- Around line 74-83: The current loop that builds bind-path strings from
bindPaths (using bind.src, bind.dst, bind.mode, bind.recursive and then
args.push("--bind-paths", path)) can produce ambiguous CLI args when src/dst
contain ',' or '='; replace the ad-hoc comma/equal concatenation with a robust,
unambiguous encoding: validate bind.src and bind.dst for emptiness, then
serialize the entire bind object (e.g., JSON.stringify(bind) or base64 of the
JSON) and pass that single encoded string to args.push("--bind-paths",
<encoded>); update the consumer that parses the flag to decode the same format,
or alternatively explicitly reject src/dst that contain ',' or '=' with a clear
error message to avoid ambiguous parsing.
| for (const bind of bindPaths ?? []) { | ||
| let path = `src=${bind.src},dst=${bind.dst}`; | ||
| if (bind.mode) { | ||
| path += `,${bind.mode}`; | ||
| } | ||
| if (bind.recursive !== undefined) { | ||
| path += `,${bind.recursive ? "recursive" : "nonrecursive"}`; | ||
| } | ||
|
|
||
| args.push("--bind-paths", path); | ||
| } |
There was a problem hiding this comment.
建议校验 src/dst 中的分隔符,避免 CLI 解析歧义。
当前拼接格式基于逗号与等号分隔,若路径包含这些字符会导致参数被错误拆分。
建议修复
for (const bind of bindPaths ?? []) {
+ if (/[=,]/.test(bind.src) || /[=,]/.test(bind.dst)) {
+ throw new Error("bindPaths src/dst 不能包含 ',' 或 '='");
+ }
let path = `src=${bind.src},dst=${bind.dst}`;
if (bind.mode) {
path += `,${bind.mode}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const bind of bindPaths ?? []) { | |
| let path = `src=${bind.src},dst=${bind.dst}`; | |
| if (bind.mode) { | |
| path += `,${bind.mode}`; | |
| } | |
| if (bind.recursive !== undefined) { | |
| path += `,${bind.recursive ? "recursive" : "nonrecursive"}`; | |
| } | |
| args.push("--bind-paths", path); | |
| } | |
| for (const bind of bindPaths ?? []) { | |
| if (/[=,]/.test(bind.src) || /[=,]/.test(bind.dst)) { | |
| throw new Error("bindPaths src/dst 不能包含 ',' 或 '='"); | |
| } | |
| let path = `src=${bind.src},dst=${bind.dst}`; | |
| if (bind.mode) { | |
| path += `,${bind.mode}`; | |
| } | |
| if (bind.recursive !== undefined) { | |
| path += `,${bind.recursive ? "recursive" : "nonrecursive"}`; | |
| } | |
| args.push("--bind-paths", path); | |
| } |
🤖 Prompt for AI Agents
In `@packages/oocana/src/layer.ts` around lines 74 - 83, The current loop that
builds bind-path strings from bindPaths (using bind.src, bind.dst, bind.mode,
bind.recursive and then args.push("--bind-paths", path)) can produce ambiguous
CLI args when src/dst contain ',' or '='; replace the ad-hoc comma/equal
concatenation with a robust, unambiguous encoding: validate bind.src and
bind.dst for emptiness, then serialize the entire bind object (e.g.,
JSON.stringify(bind) or base64 of the JSON) and pass that single encoded string
to args.push("--bind-paths", <encoded>); update the consumer that parses the
flag to decode the same format, or alternatively explicitly reject src/dst that
contain ',' or '=' with a clear error message to avoid ambiguous parsing.
Summary
bind_paths?: string[]withbindPaths?: BindPath[]increatePackageLayerbindPathPatternregex validationbuildRunConfigArgsinoocana.tsTest plan
pnpm buildpassespnpm testpasses