fix(market): include plugin_dependencies in build_market_plugin_payload - #3
Open
fuilyha56-wq wants to merge 1 commit into
Open
fuilyha56-wq wants to merge 1 commit into
fuilyha56-wq wants to merge 1 commit into
Conversation
ManifestManager.build_market_plugin_payload() did not map
manifest.dependencies.plugins to the PluginCreate.plugin_dependencies
field of the market register API. As a result, plugins published via
'mpdt market publish' registered with an empty dependencies endpoint
(/api/v1/plugins/{id}/dependencies), and authors had to PUT the
dependencies manually after every first publish.
Reuse the existing get_plugin_dependencies() helper so the dependency
specs (e.g. 'onebot_expand>=1.0.13') are forwarded during registration.
Defaults to an empty list when dependencies.plugins is absent, matching
the OpenAPI schema default.
Add unit tests covering both the populated and empty-dependency cases.
Reviewer's GuideAdds propagation of manifest-declared plugin dependencies into the market plugin registration payload and covers it with tests to ensure correct transmission and default behavior when dependencies are absent. Sequence diagram for including plugin_dependencies in market plugin payloadsequenceDiagram
actor User
participant MpdtCLI
participant ManifestManager
participant MarketAPI
User->>MpdtCLI: mpdt_market_publish
MpdtCLI->>ManifestManager: build_market_plugin_payload
ManifestManager->>ManifestManager: get_plugin_dependencies
ManifestManager-->>MpdtCLI: payload_with_plugin_dependencies
MpdtCLI->>MarketAPI: POST_api_v1_plugins(payload_with_plugin_dependencies)
MarketAPI-->>MpdtCLI: PluginCreate_response
Note over MarketAPI: plugin_dependencies stored
User->>MarketAPI: GET_api_v1_plugins_plugin_id_dependencies
MarketAPI-->>User: plugin_dependencies_list
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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="tests/test_manifest_manager.py" line_range="55-64" />
<code_context>
+ )
+
+ manager = ManifestManager(plugin_dir)
+ payload = manager.build_market_plugin_payload(
+ repository_url="https://github.com/test/deps_plugin"
+ )
+
+ assert payload["plugin_dependencies"] == [
+ "onebot_expand>=1.0.13",
+ "neo_fatum_chatter",
+ ]
+
+
+def test_build_market_plugin_payload_defaults_dependencies_to_empty(tmp_path) -> None:
+ plugin_dir = tmp_path / "no_deps_plugin"
+ plugin_dir.mkdir()
+ (plugin_dir / "manifest.json").write_text(
+ json.dumps(
+ {
+ "name": "no_deps_plugin",
+ "version": "1.0.0",
+ "description": "plugin without deps",
+ "author": "tester",
+ }
+ ),
+ encoding="utf-8",
+ )
+
+ manager = ManifestManager(plugin_dir)
+ payload = manager.build_market_plugin_payload(
+ repository_url="https://github.com/test/no_deps_plugin"
+ )
+
</code_context>
<issue_to_address>
**issue (testing):** Add an assertion to verify `plugin_dependencies` defaults to an empty list
This test constructs the payload but never checks `plugin_dependencies`. Please add an assertion that it defaults to an empty list so the regression is explicitly covered, e.g.:
```python
assert payload["plugin_dependencies"] == []
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+55
to
+64
| payload = manager.build_market_plugin_payload( | ||
| repository_url="https://github.com/test/deps_plugin" | ||
| ) | ||
|
|
||
| assert payload["plugin_dependencies"] == [ | ||
| "onebot_expand>=1.0.13", | ||
| "neo_fatum_chatter", | ||
| ] | ||
|
|
||
|
|
There was a problem hiding this comment.
issue (testing): Add an assertion to verify plugin_dependencies defaults to an empty list
This test constructs the payload but never checks plugin_dependencies. Please add an assertion that it defaults to an empty list so the regression is explicitly covered, e.g.:
assert payload["plugin_dependencies"] == []
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.
问题
mpdt market publish发布插件后,市场公开端点/api/v1/plugins/{plugin_id}/dependencies一直为空,作者必须在首次发布后手动PUT /api/v1/plugins/{plugin_id}提交plugin_dependencies才能让详情页显示依赖约束。已确认根因:
ManifestManager.build_market_plugin_payload()构建的注册 payload 没有包含manifest.dependencies.plugins,尽管:get_plugin_dependencies()辅助方法;PluginCreateschema 明确接受plugin_dependencies: array[string]字段;PluginUpdate也接受plugin_dependencies;/api/v1/plugins/{plugin_id}/dependencies读取该字段。影响版本:0.6.5 及当前 master(0.6.6)均存在。
修复
在
build_market_plugin_payload()的返回字典中新增plugin_dependencies,复用已有的get_plugin_dependencies()读取manifest.dependencies.plugins。缺失时返回空列表,与 OpenAPI schema 默认值一致。测试
新增两条单测覆盖:
test_build_market_plugin_payload_includes_plugin_dependencies:manifest 声明dependencies.plugins时 payload 正确透传(含版本约束>=1.0.13)。test_build_market_plugin_payload_defaults_dependencies_to_empty:manifest 无dependencies时 payload 中plugin_dependencies == []。ruff check tests/test_manifest_manager.py通过;manifest_manager.py的既有 W292 报错与本次改动无关(均为模板字典/文档字符串,master 上已存在)。兼容性
PluginCreate已接受该字段,注册请求无需额外变更;package-update仍走版本提交路径;若需同步已有条目的依赖,可继续用PUT或mpdt market后续命令补充(本次不改动 package-update 路径,避免扩大改动面)。Summary by Sourcery
Include plugin dependency metadata in market plugin registration payload and add tests to cover the new behavior.
Bug Fixes:
Tests: