Merged
Conversation
- 在 Toolbar.tsx 的 startTasksForInstance 中添加任务兼容性过滤 - 只有与当前控制器和资源兼容的任务才会被提交执行 - 当任务被跳过时，在日志中记录详细的警告信息 - 添加 i18n 字符串以支持跳过任务的通知消息 修复了任务显示"不支持当前控制器"警告但仍会执行的问题。 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈：
- 新的
skippedTasks计算依赖于在任务对象上使用Array.prototype.includes，这样会有点脆弱；为了避免在数组被重新创建或任务被克隆时出现问题，建议改为通过稳定的标识符（例如id）进行比较。 - 日志消息
实例 ${targetInstance.name} 没有兼容当前控制器和资源的任务是硬编码的，而不是像其他新增日志消息那样使用 i18n 系统；为了保持一致性，建议为其添加一个翻译键。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The new `skippedTasks` calculation relies on `Array.prototype.includes` with task objects, which is a bit brittle; prefer comparing by a stable identifier (e.g., `id`) to avoid issues if arrays are recreated or tasks are cloned.
- The message `实例 ${targetInstance.name} 没有兼容当前控制器和资源的任务` is hard-coded instead of using the i18n system like the other new log messages; consider adding a translation key for consistency.帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- The new
skippedTaskscalculation relies onArray.prototype.includeswith task objects, which is a bit brittle; prefer comparing by a stable identifier (e.g.,id) to avoid issues if arrays are recreated or tasks are cloned. - The message
实例 ${targetInstance.name} 没有兼容当前控制器和资源的任务is hard-coded instead of using the i18n system like the other new log messages; consider adding a translation key for consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `skippedTasks` calculation relies on `Array.prototype.includes` with task objects, which is a bit brittle; prefer comparing by a stable identifier (e.g., `id`) to avoid issues if arrays are recreated or tasks are cloned.
- The message `实例 ${targetInstance.name} 没有兼容当前控制器和资源的任务` is hard-coded instead of using the i18n system like the other new log messages; consider adding a translation key for consistency.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体反馈：
- 当前兼容性过滤在多个地方（filter、跳过日志、mapping）都通过
projectInterface?.task.find(...)做重复的线性查找。当启用了很多 task 时，这会导致 O(n²) 的行为，可以通过预先计算一个Map<taskName, taskDef>来一次性完成查找、避免多次线性遍历。 - 变量名
compatibleTaskIds被同时用于两个不同的含义：在计算skippedTasks时表示 ID 的Set，而之后又作为传给setAllTasksRunStatus的 ID 数组。重命名其中一个会让代码更清晰，也能避免混淆。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The compatibility filtering does repeated linear lookups with `projectInterface?.task.find(...)` in multiple places (filter, skipped logging, mapping), which can be turned into a single precomputed `Map<taskName, taskDef>` to avoid O(n²) behavior when many tasks are enabled.
- The variable name `compatibleTaskIds` is used both for the `Set` of IDs when computing `skippedTasks` and later for an array of IDs passed to `setAllTasksRunStatus`; renaming one of them would make the code clearer and avoid confusion.帮我变得更有用！请在每条评论上点击 👍 或 👎，我会根据你的反馈改进之后的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- The compatibility filtering does repeated linear lookups with
projectInterface?.task.find(...)in multiple places (filter, skipped logging, mapping), which can be turned into a single precomputedMap<taskName, taskDef>to avoid O(n²) behavior when many tasks are enabled. - The variable name
compatibleTaskIdsis used both for theSetof IDs when computingskippedTasksand later for an array of IDs passed tosetAllTasksRunStatus; renaming one of them would make the code clearer and avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The compatibility filtering does repeated linear lookups with `projectInterface?.task.find(...)` in multiple places (filter, skipped logging, mapping), which can be turned into a single precomputed `Map<taskName, taskDef>` to avoid O(n²) behavior when many tasks are enabled.
- The variable name `compatibleTaskIds` is used both for the `Set` of IDs when computing `skippedTasks` and later for an array of IDs passed to `setAllTasksRunStatus`; renaming one of them would make the code clearer and avoid confusion.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes task execution so that tasks incompatible with the currently selected controller/resource are skipped, and task startup is aborted when no compatible tasks remain—preventing “unsupported controller/resource” tasks from still running (Fix #153).
Changes:
- Filter enabled tasks to a compatible subset before building configs, initializing run state, and registering task ID mappings.
- Add warning messages for skipped incompatible tasks (including controller/resource-specific reasons) and for “no compatible tasks”.
- Add EN/ZH translation strings for the new messages.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/components/Toolbar.tsx | Filters incompatible tasks before starting and logs skip/abort reasons; switches startup/status/mapping logic to the filtered set. |
| src/i18n/locales/en-US.ts | Adds English strings describing skipped incompatible tasks and “no compatible tasks”. |
| src/i18n/locales/zh-CN.ts | Adds Chinese strings describing skipped incompatible tasks and “no compatible tasks”. |
Comments suppressed due to low confidence (1)
src/components/Toolbar.tsx:786
taskConfigsis built by iteratingcompatibleTasks, butif (!taskDef) continue;can drop entries. This can desync the order/length betweentaskConfigs/taskIdsandcompatibleTasks, which later code assumes are index-aligned for status init and ID mapping. Consider constructing a singlerunnableTasksarray (with resolved taskDef/entry) and using that consistently fortaskConfigs, status initialization, and mapping.
for (const selectedTask of compatibleTasks) {
// 先检查是否是 MXU 特殊任务
const specialTask = getMxuSpecialTask(selectedTask.taskName);
const taskDef =
specialTask?.taskDef ||
projectInterface?.task.find((t) => t.name === selectedTask.taskName);
if (!taskDef) continue;
taskConfigs.push({
构建 runnableTasks 数组以确保 taskConfigs、taskIds 和任务映射之间的索引对齐。 这解决了当某些兼容任务缺少 taskDef 时可能导致的索引不匹配问题。 变更： - 创建 RunnableTask 接口，包含 selectedTask、taskDef 和 specialTask - 在构建 taskConfigs 前先过滤掉没有 taskDef 的任务 - 使用 runnableTasks 进行状态初始化和 ID 映射，确保索引一致 - 添加缺少 taskDef 时的警告日志 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
修复问题：
- 将所有 t('action.*') 调用更正为 t('taskList.*') 以匹配实际的 i18n 键定义
- tasksSkippedDueToIncompatibility: action.* → taskList.*
- taskSkippedController: action.* → taskList.*
- taskSkippedResource: action.* → taskList.*
- noCompatibleTasks: action.* → taskList.*
改进用户体验：
- 当任务被跳过时，向用户显示警告消息（addLog warning）
- 当没有兼容任务时，向用户显示明确的错误消息（addLog error）
- 避免用户只看到通用的 'startFailed' 错误
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
192
to
+200
| // 获取控制器和资源配置 | ||
| const controllerName = selectedController[targetId] || projectInterface?.controller[0]?.name; | ||
| const resourceName = selectedResource[targetId] || projectInterface?.resource[0]?.name; | ||
|
|
||
| // 过滤掉不兼容当前控制器/资源的任务 | ||
| const compatibleTasks = enabledTasks.filter((t) => { | ||
| const taskDef = projectInterface?.task.find((td) => td.name === t.taskName); | ||
| return isTaskCompatible(taskDef, controllerName, resourceName); | ||
| }); |
There was a problem hiding this comment.
startTasksForInstance resolves controllerName/resourceName using only selectedController/selectedResource (or PI defaults), but elsewhere in this component you also fall back to instance.controllerName/resourceName. If selectedController[targetId] is unset (e.g. after reload) this can pick the wrong controller/resource, causing compatibility filtering (and pipeline overrides) to be computed against the wrong selection and potentially still run “incompatible” tasks. Align the fallback order with the rest of the file (or reuse getCurrentControllerAndResource).
Comment on lines
+216
to
+218
| const taskLabel = taskDef?.label | ||
| ? resolveI18nText(taskDef.label, translations) | ||
| : task.taskName; |
There was a problem hiding this comment.
Skipped-task labeling ignores selectedTask.customName. If a user renamed a task, the warning will show the PI label/taskName instead of the name shown elsewhere (e.g. when registering task display names later in this function). Consider preferring task.customName (and keeping MXU-special-task translation behavior consistent) so the skip message refers to the same display name the user sees in the task list.
Suggested change
| const taskLabel = taskDef?.label | |
| ? resolveI18nText(taskDef.label, translations) | |
| : task.taskName; | |
| const mxuSpecialTask = getMxuSpecialTask(task.taskName); | |
| const taskLabel = task.customName | |
| ? task.customName | |
| : mxuSpecialTask | |
| ? t(mxuSpecialTask.translationKey) | |
| : taskDef?.label | |
| ? resolveI18nText(taskDef.label, translations) | |
| : task.taskName; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #153
Summary by Sourcery
跳过与所选控制器/资源不兼容的任务，并确保任务执行仅在剩余的兼容任务上运行。
错误修复：
改进：
文档：
Original summary in English
Summary by Sourcery
Skip tasks incompatible with the selected controller/resource and ensure task execution only runs on the remaining compatible tasks.
Bug Fixes:
Enhancements:
Documentation:
Bug 修复：
增强功能：
文档：
Original summary in English
Summary by Sourcery
跳过与所选控制器/资源不兼容的任务，并确保任务执行仅在剩余的兼容任务上运行。
错误修复：
改进：
文档：
Original summary in English
Summary by Sourcery
Skip tasks incompatible with the selected controller/resource and ensure task execution only runs on the remaining compatible tasks.
Bug Fixes:
Enhancements:
Documentation: