feat(subagent): add JSON import/export actions for SubAgent config#6913
feat(subagent): add JSON import/export actions for SubAgent config#6913Jacobinwwey wants to merge 4 commits intoAstrBotDevs:masterfrom
Conversation
|
Warning Gemini encountered an error creating the summary. You can try again by commenting |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
handleImportFile, the validation only checks for a non-array object but does not enforce the "non-empty object" constraint described in the PR text, so consider also rejecting empty objects to match the intended behavior. - When importing JSON, it may be worth adding a simple file size guard (e.g., a max size in MB) before reading the file to avoid UI freezes or memory issues with unexpectedly large uploads.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `handleImportFile`, the validation only checks for a non-array object but does not enforce the "non-empty object" constraint described in the PR text, so consider also rejecting empty objects to match the intended behavior.
- When importing JSON, it may be worth adding a simple file size guard (e.g., a max size in MB) before reading the file to avoid UI freezes or memory issues with unexpectedly large uploads.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/SubAgentPage.vue" line_range="394-412" />
<code_context>
+ }
+}
+
+function exportConfig() {
+ try {
+ const payload = toPersistedConfig(cfg.value)
+ const json = JSON.stringify(payload, null, 2)
+ const blob = new Blob([json], { type: 'application/json;charset=utf-8' })
+ const url = URL.createObjectURL(blob)
+ const link = document.createElement('a')
+ const date = new Date().toISOString().slice(0, 10)
+ link.href = url
+ link.download = `subagent-config-${date}.json`
+ document.body.appendChild(link)
+ link.click()
+ document.body.removeChild(link)
+ URL.revokeObjectURL(url)
+ toast(tm('messages.exportSuccess'), 'success')
+ } catch (e: unknown) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The object URL is only revoked in the success path; moving it into a `finally` would avoid leaks on unexpected errors.
Because `revokeObjectURL` is only reached on the happy path, any exception during `link.click()` or DOM manipulation means the URL is never revoked. Please move `URL.revokeObjectURL(url)` into a `finally` (and keep `createObjectURL` scoped with it) so the URL is always cleaned up, even on errors.
```suggestion
function exportConfig() {
let url: string | null = null
try {
const payload = toPersistedConfig(cfg.value)
const json = JSON.stringify(payload, null, 2)
const blob = new Blob([json], { type: 'application/json;charset=utf-8' })
url = URL.createObjectURL(blob)
const link = document.createElement('a')
const date = new Date().toISOString().slice(0, 10)
link.href = url
link.download = `subagent-config-${date}.json`
document.body.appendChild(link)
link.click()
document.body.removeChild(link)
toast(tm('messages.exportSuccess'), 'success')
} catch (e: unknown) {
toast(tm('messages.exportFailed'), 'error')
} finally {
if (url) {
URL.revokeObjectURL(url)
}
}
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Related Documentation 1 document(s) may need updating based on files changed in this PR: AstrBotTeam's Space pr4697的改动View Suggested Changes@@ -666,6 +666,12 @@
- 配置持久化
- 供应商类型选择:现在支持在“选择供应商”下拉菜单中选择 `chat_completion` 和 `agent_runner` 类型,便于根据不同需求配置子代理的执行方式。
- 页面副标题已更新为:“主 LLM 可直接使用自身工具,也可通过 handoff 分派给各个 SubAgent。”
+- **配置导入/导出**([PR #6913](https://github.com/AstrBotDevs/AstrBot/pull/6913)):页面顶部操作区新增"导入 JSON"和"导出 JSON"按钮,方便配置备份、跨环境迁移和团队共享:
+ - **导出 JSON**:将当前页面配置导出为本地 JSON 文件,便于备份
+ - **导入 JSON**:从本地 JSON 文件导入配置并自动回填页面表单
+ - **安全验证**:导入功能包含基础安全校验,文件内容必须为有效 JSON 对象(非数组、非空对象),无效文件会显示错误提示并拒绝加载
+ - **向后兼容**:导入功能支持旧配置文件,旧字段 `enable` 会自动转换为 `main_enable`
+ - **统一序列化**:导出和保存功能使用相同的配置结构(`toPersistedConfig`),防止格式漂移
主代理和子代理的工具分配逻辑在 UI 中有清晰展示,支持灵活配置。
Note: You must be authenticated to accept/decline updates. |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
toPersistedConfighelper changes behavior compared to the previous save payload (e.g., trimmingname/persona_idand defaultingenabledtotruewhenundefined); if this is not intentional, consider aligning the serialization logic with the prior structure to avoid subtle config mutations on save/export. - In
handleImportFile, you only validate that the parsed JSON is a non-array object; consider adding a minimal structural check (e.g., presence and types ofmain_enable,agents, and required agent fields) so malformed but syntactically valid objects don’t silently overwrite the current configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `toPersistedConfig` helper changes behavior compared to the previous save payload (e.g., trimming `name`/`persona_id` and defaulting `enabled` to `true` when `undefined`); if this is not intentional, consider aligning the serialization logic with the prior structure to avoid subtle config mutations on save/export.
- In `handleImportFile`, you only validate that the parsed JSON is a non-array object; consider adding a minimal structural check (e.g., presence and types of `main_enable`, `agents`, and required agent fields) so malformed but syntactically valid objects don’t silently overwrite the current configuration.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/SubAgentPage.vue" line_range="380-388" />
<code_context>
cfg.value.agents.splice(idx, 1)
}
+function toPersistedConfig(source: SubAgentConfig) {
+ return {
+ main_enable: !!source.main_enable,
+ remove_main_duplicate_tools: !!source.remove_main_duplicate_tools,
+ agents: source.agents.map((a) => ({
+ name: (a.name || '').trim(),
+ persona_id: (a.persona_id || '').trim(),
+ public_description: a.public_description || '',
+ enabled: a.enabled !== false,
+ provider_id: a.provider_id
+ }))
</code_context>
<issue_to_address>
**issue (bug_risk):** The `enabled: a.enabled !== false` coercion changes semantics compared to the previous `enabled: a.enabled` behavior.
This will treat any non-`false` value (`undefined`, `null`, `0`, `''`) as `true`, so agents without an explicit `enabled` flag will now be persisted as enabled. If you want boolean normalization, consider `enabled: !!a.enabled` (falsy → disabled) or only default when `enabled` is `undefined`, e.g. `enabled: a.enabled === undefined ? true : !!a.enabled`, depending on the desired behavior for existing data.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/views/SubAgentPage.vue" line_range="387-389" />
<code_context>
cfg.value.agents.splice(idx, 1)
}
+function toPersistedConfig(source: SubAgentConfig) {
+ return {
+ main_enable: !!source.main_enable,
+ remove_main_duplicate_tools: !!source.remove_main_duplicate_tools,
+ agents: source.agents.map((a) => ({
+ name: (a.name || '').trim(),
+ persona_id: (a.persona_id || '').trim(),
+ public_description: a.public_description || '',
+ enabled: a.enabled,
+ provider_id: a.provider_id
+ }))
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider normalizing `enabled` to a boolean in the persisted payload.
In `toPersistedConfig`, other flags are coerced to booleans, but `enabled` is not. If `SubAgentConfig.enabled` is ever `null`/`undefined` or another truthy/falsy value (e.g., from older configs or UI issues), the persisted payload may not match backend expectations. Coercing here (e.g., `enabled: !!a.enabled`) would keep this consistent with `main_enable`/`remove_main_duplicate_tools` and make saves/imports more robust.
```suggestion
public_description: a.public_description || '',
enabled: !!a.enabled,
provider_id: a.provider_id
```
</issue_to_address>Hi @Jacobinwwey! 👋
Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀
Install the sourcery-ai bot to get automatic code reviews on every pull request ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.| public_description: a.public_description || '', | ||
| enabled: a.enabled, | ||
| provider_id: a.provider_id |
There was a problem hiding this comment.
suggestion (bug_risk): Consider normalizing enabled to a boolean in the persisted payload.
In toPersistedConfig, other flags are coerced to booleans, but enabled is not. If SubAgentConfig.enabled is ever null/undefined or another truthy/falsy value (e.g., from older configs or UI issues), the persisted payload may not match backend expectations. Coercing here (e.g., enabled: !!a.enabled) would keep this consistent with main_enable/remove_main_duplicate_tools and make saves/imports more robust.
| public_description: a.public_description || '', | |
| enabled: a.enabled, | |
| provider_id: a.provider_id | |
| public_description: a.public_description || '', | |
| enabled: !!a.enabled, | |
| provider_id: a.provider_id |
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
normalizeConfig, the fallback logic formain_enable(raw?.main_enable !== undefined ? !!raw.main_enable : !!raw?.enable) will treat anullvalue as ifmain_enablewere absent and fall back toenable; if you want to strictly prefermain_enablewhenever it is present (even whennull), consider using a property check like('main_enable' in raw)instead of comparing toundefined. - The
toPersistedConfighelper trimsnameandpersona_idbut not other string fields; if leading/trailing whitespace inpublic_descriptionor other string attributes should also be normalized for consistency, consider applying similar trimming there or centralizing normalization in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `normalizeConfig`, the fallback logic for `main_enable` (`raw?.main_enable !== undefined ? !!raw.main_enable : !!raw?.enable`) will treat a `null` value as if `main_enable` were absent and fall back to `enable`; if you want to strictly prefer `main_enable` whenever it is present (even when `null`), consider using a property check like `('main_enable' in raw)` instead of comparing to `undefined`.
- The `toPersistedConfig` helper trims `name` and `persona_id` but not other string fields; if leading/trailing whitespace in `public_description` or other string attributes should also be normalized for consistency, consider applying similar trimming there or centralizing normalization in one place.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/SubAgentPage.vue" line_range="431-439" />
<code_context>
+
+ try {
+ const text = await file.text()
+ const parsed = JSON.parse(text) as unknown
+ if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
+ toast(tm('messages.importInvalidJson'), 'error')
+ return
</code_context>
<issue_to_address>
**suggestion:** Consider validating the expected config shape more specifically before passing it into `normalizeConfig`.
Right now any non-array object passes (e.g. `{ foo: 1 }`), which can produce surprising defaults or an effectively empty config. It would help to require at least one expected key (like `main_enable`/`enable` or `agents`) and show `importInvalidJson` when the structure doesn’t match a subagent config, so invalid imports fail explicitly instead of being silently accepted.
```suggestion
try {
const text = await file.text()
const parsed = JSON.parse(text) as unknown
if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) {
toast(tm('messages.importInvalidJson'), 'error')
return
}
const obj = parsed as Record<string, unknown>
const hasAgents = Array.isArray(obj['agents'] as unknown)
const hasEnableFlag =
typeof obj['main_enable'] === 'boolean' || typeof obj['enable'] === 'boolean'
if (!hasAgents && !hasEnableFlag) {
toast(tm('messages.importInvalidJson'), 'error')
return
}
cfg.value = normalizeConfig(parsed)
toast(tm('messages.importSuccess'), 'success')
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
|
[Passed the review by Sourcery] |
Motivation / 动机
为 SubAgent 编排页补充“导入/导出 JSON”能力,方便配置备份、跨环境迁移和团队共享,不再依赖手动复制粘贴。
Modifications / 改动点
前端功能新增:
SubAgentPage顶部操作区新增两个按钮:Export JSON:导出当前页面配置为本地 JSON 文件。Import JSON:从本地 JSON 文件导入配置并回填页面表单。toPersistedConfig,让“保存到后端”和“导出 JSON”使用同一配置结构,降低格式漂移风险。兼容性处理:
normalizeConfig支持兼容旧字段enable -> main_enable,导入旧配置文件时可平滑兼容。i18n 文案补充:
新增
zh-CN / en-US / ru-RU三语文案键:import,exportimportSuccess,importFailed,importInvalidJson,exportSuccess,exportFailedThis is NOT a breaking change. / 这不是一个破坏性变更。
Test Results / 测试结果
已在最新
master基线上完成本地校验:npm --prefix dashboard run typecheck✅ 通过手动验证路径:
Export JSON,确认下载文件内容与当前配置一致。Import JSON导入刚导出的文件,确认配置正确回填。Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.🤖 Generated with Codex
Summary by Sourcery
Add JSON import and export capabilities for SubAgent configuration and align persistence format across save and export flows.
New Features:
Enhancements: