Conversation
There was a problem hiding this comment.
嘿，我已经审查了你的修改，看起来很棒！
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Added comments in SubTask.json for clarity on fields.
There was a problem hiding this comment.
Pull request overview
This PR introduces a new generic SubTask custom action for the Go-based agent service. The action sequentially runs a list of pipeline task names specified in custom_action_param.sub, with configurable behavior on sub-task failure (continue to keep going, strict to propagate failure). This enables pipeline JSON authors to compose sequential sub-task execution without writing custom Go code for each composition.
Changes:
- Added
subtaskGo package withSubTaskActionimplementation that sequentially runs pipeline tasks with configurable failure handling - Registered the new
SubTaskcustom action in the mainregisterAll()function, reorganizing the registration into logical groups (Pre-Check, General, Business) - Added an example pipeline JSON demonstrating the
SubTaskcustom action usage
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
agent/go-service/subtask/action.go |
Core implementation of SubTaskAction with param parsing, sequential task execution, and configurable failure modes (continue/strict) |
agent/go-service/subtask/register.go |
Registration file with compile-time interface check and Register() function |
agent/go-service/register.go |
Updated registerAll() to include subtask.Register() and reorganized registrations into logical groups |
assets/resource/pipeline/Interface/Example/SubTask.json |
Example pipeline JSON demonstrating the SubTask custom action with two example steps |
| _ maa.CustomActionRunner = &SubTaskAction{} | ||
| ) | ||
|
|
||
| func Register() { |
There was a problem hiding this comment.
Missing doc comment on the exported Register() function. Every other package in this codebase consistently provides a doc comment starting with the function name (e.g., // Register registers all custom action components for subtask package). See importtask/register.go:11, autofight/register.go:13, charactercontroller/register.go:12 for examples.
| Strict *bool `json:"strict,omitempty"` | ||
| } | ||
|
|
||
| type SubTaskAction struct{} |
There was a problem hiding this comment.
Per the Go service coding guidelines, exported types should have a doc comment explaining their purpose. Consider adding a comment like // SubTaskAction is a custom action that sequentially runs a list of pipeline sub-tasks. before the type definition. While not all existing packages follow this consistently (e.g., charactercontroller), packages like essencefilter do (see essencefilter/actions.go:18).
agent/go-service/register.go
Outdated
| "github.com/MaaXYZ/MaaEnd/agent/go-service/hdrcheck" | ||
| "github.com/MaaXYZ/MaaEnd/agent/go-service/subtask" |
There was a problem hiding this comment.
The imports are no longer in alphabetical order. Go convention (and the project's go fmt formatter) expects imports within a single group to be sorted alphabetically. hdrcheck and subtask have been moved above autofight, breaking the sort order. Either keep all imports in one alphabetically-sorted group, or if you want logical grouping to match the registerAll() function body, separate them with blank lines into distinct import groups.
| "github.com/MaaXYZ/MaaEnd/agent/go-service/hdrcheck" | |
| "github.com/MaaXYZ/MaaEnd/agent/go-service/subtask" | |
| "github.com/MaaXYZ/MaaEnd/agent/go-service/hdrcheck" | |
| "github.com/MaaXYZ/MaaEnd/agent/go-service/subtask" |
agent/go-service/register.go
Outdated
| // Pre-Check Custom | ||
| aspectratio.Register() | ||
| hdrcheck.Register() | ||
|
|
||
| // General Custom | ||
| subtask.Register() | ||
|
|
||
| // Business Custom |
There was a problem hiding this comment.
These comment lines use spaces for indentation instead of tabs. Go source files must use tabs for indentation (enforced by go fmt). The comments on lines 20, 24, and 27 each have 4 spaces instead of a tab character.
| // Pre-Check Custom | |
| aspectratio.Register() | |
| hdrcheck.Register() | |
| // General Custom | |
| subtask.Register() | |
| // Business Custom | |
| // Pre-Check Custom | |
| aspectratio.Register() | |
| hdrcheck.Register() | |
| // General Custom | |
| subtask.Register() | |
| // Business Custom |
加了个 SubTask.json 的例子，可以看看
{ "SubTaskExampleRun": { "desc": "SubTask 使用示例：按顺序执行 custom_action_param.sub 中的任务名", "recognition": "DirectHit", "action": "Custom", "custom_action": "SubTask", "custom_action_param": { "sub": [ "SubTaskExampleStepOne", "SubTaskExampleStepTwo" ], // 任一 sub 失败，是否继续执行后续 sub。可选字段，默认 false "continue": false, // 任一 sub 失败，是否 action 视为失败。可选字段，默认 true "strict": true }, "next": [] }, "SubTaskExampleStepOne": { "desc": "SubTask 子任务 1", "recognition": "DirectHit", "action": "DoNothing", "next": [] }, "SubTaskExampleStepTwo": { "desc": "SubTask 子任务 2", "recognition": "DirectHit", "action": "DoNothing", "next": [] } }Summary by Sourcery
在代理服务中引入一个新的 SubTask 自定义动作，用于运行一系列子任务，并将其注册到代理服务中。
新功能：
Original summary in English
Summary by Sourcery
Introduce a new SubTask custom action for running sequences of subtasks and register it in the agent service.
New Features: