refactor: embed agent runner configuration in profiles - #9821
Conversation
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="astrbot/core/config/astrbot_config.py" line_range="196-198" />
<code_context>
# 类型不匹配,使用默认值
new_conf[key] = value
has_new = True
+ elif (path + "." + key if path else key) == "agent_runner.config":
+ # Runner config is normalized according to runner_type when saved.
+ new_conf[key] = conf[key]
else:
# 递归检查并同步顺序
</code_context>
<issue_to_address>
**issue (bug_risk):** A persisted `agent_runner` object with a malformed or incomplete `config` bypasses recursive integrity checking and is copied unchanged. `InternalAgentSubStage.initialize` then indexes `runner_config["model"]`, `runner_config["persona"]`, `runner_config["compression"]`, and `runner_config["misc"]`, so such a configuration raises `KeyError` during pipeline initialization instead of being repaired with runner defaults.
**Triggers:** When an existing profile contains an incomplete `agent_runner.config`, such as after manual editing or a partial migration.
**Suggested fix:** Normalize `agent_runner` with `normalize_agent_runner` before accepting it in `check_config_integrity`, or ensure every consumer handles missing nested sections.
</issue_to_address>
### Comment 2
<location path="astrbot/core/config/agent_runner.py" line_range="213-224" />
<code_context>
+ return None
+
+
+def _copy_provider_config(
+ runner_type: str,
+ provider: dict[str, Any],
+) -> dict[str, Any]:
+ config = {
+ key: copy.deepcopy(value)
+ for key, value in provider.items()
+ if key not in _LEGACY_PROVIDER_IDENTITY_FIELDS
+ }
+ return normalize_agent_runner({"runner_type": runner_type, "config": config})[
+ "config"
+ ]
+
+
</code_context>
<issue_to_address>
**issue (broader_impact):** Migration copies the raw legacy runner provider dictionary directly into the embedded profile configuration and removes provider identity fields, but it never merges the provider's `provider_source_id` source configuration. Legacy runner settings stored in a provider source therefore disappear during migration, leaving the embedded runner with defaults or incomplete credentials.
**Triggers:** When a legacy Dify, Coze, DashScope, or DeerFlow provider uses `provider_source_id` for API settings.
**Suggested fix:** Resolve the provider through the same merged-provider path used at runtime before passing it to `_copy_provider_config`, or explicitly merge the referenced provider source during migration.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and this migration rewrites persisted profile settings, embeds runner credentials, and removes the old agent-runner provider records; an incorrect mapping could alter future requests or lose stored configuration. Reverting the code would not restore already-saved profiles or deleted provider records.
Blocking findings: astrbot/core/config/astrbot_config.py:198, astrbot/core/config/agent_runner.py:224
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| elif (path + "." + key if path else key) == "agent_runner.config": | ||
| # Runner config is normalized according to runner_type when saved. | ||
| new_conf[key] = conf[key] |
There was a problem hiding this comment.
issue (bug_risk): A persisted agent_runner object with a malformed or incomplete config bypasses recursive integrity checking and is copied unchanged. InternalAgentSubStage.initialize then indexes runner_config["model"], runner_config["persona"], runner_config["compression"], and runner_config["misc"], so such a configuration raises KeyError during pipeline initialization instead of being repaired with runner defaults.
Triggers: When an existing profile contains an incomplete agent_runner.config, such as after manual editing or a partial migration.
Suggested fix: Normalize agent_runner with normalize_agent_runner before accepting it in check_config_integrity, or ensure every consumer handles missing nested sections.
| def _copy_provider_config( | ||
| runner_type: str, | ||
| provider: dict[str, Any], | ||
| ) -> dict[str, Any]: | ||
| config = { | ||
| key: copy.deepcopy(value) | ||
| for key, value in provider.items() | ||
| if key not in _LEGACY_PROVIDER_IDENTITY_FIELDS | ||
| } | ||
| return normalize_agent_runner({"runner_type": runner_type, "config": config})[ | ||
| "config" | ||
| ] |
There was a problem hiding this comment.
issue (broader_impact): Migration copies the raw legacy runner provider dictionary directly into the embedded profile configuration and removes provider identity fields, but it never merges the provider's provider_source_id source configuration. Legacy runner settings stored in a provider source therefore disappear during migration, leaving the embedded runner with defaults or incomplete credentials.
Triggers: When a legacy Dify, Coze, DashScope, or DeerFlow provider uses provider_source_id for API settings.
Suggested fix: Resolve the provider through the same merged-provider path used at runtime before passing it to _copy_provider_config, or explicitly merge the referenced provider source during migration.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
astrbot-docs | c985490 | Commit Preview URL Branch Preview URL |
Aug 28 2026, 09:29 AM |
1a3d1d1 to
ddb78df
Compare
ddb78df to
bea61ff
Compare
Summary
Dependency
masterValidation
uv run pytest tests/unit/test_agent_runner_config.py tests/unit/test_astrbot_config_manager.py tests/unit/test_config.py tests/unit/test_config_profile_service.py tests/unit/test_cron_manager.py tests/unit/test_astr_agent_tool_exec.py tests/test_conversation_checkpoint.py tests/test_deerflow_agent_runner.py -quv run ruff check .cd dashboard && pnpm typecheckcd dashboard && pnpm buildgit diff --checkSummary by Sourcery
Embed Agent Runner configuration in profiles and migrate existing settings away from provider-backed storage.
New Features:
Bug Fixes:
Enhancements:
Tests:
Chores: