Repository navigation
feat(plugin): support localized plugin READMEs with a dashboard switcher - #10619
antares127 wants to merge 3 commits into
Conversation
Plugins can declare readmes in metadata.yaml. The dashboard resolves the requested locale (exact, then base language, then default), validates that declared paths stay inside the plugin directory, and shows a language selector when more than one README exists.
- Identity-check fetchPluginDetail responses: a slow response for plugin A can no longer overwrite pluginDetail after the user switched to plugin B - Guard the watcher continuation so README/changelog fetches are only started when the detail request still belongs to the current selection - Extend the README request guard to also verify the plugin identity, not just the request id, and apply the same staleness checks to the market README branch which previously had none
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="astrbot/dashboard/services/plugin_service.py" line_range="2030-2037" />
<code_context>
+ candidates: list[tuple[str | None, str]] = []
+ if requested:
+ matched_locale = None
+ if requested in locale_paths:
+ matched_locale = requested
+ elif "-" in requested:
+ base_language = requested.split("-", 1)[0]
+ if base_language in locale_paths:
+ matched_locale = base_language
+ if matched_locale:
+ candidates.append((matched_locale, locale_paths[matched_locale]))
+ candidates.append((None, declared.get("default") or "README.md"))
+
</code_context>
<issue_to_address>
**Base-language fallback is skipped**
When the requested exact-locale README is declared but missing or invalid, while a base-language README and default README are available, `get_plugin_readme()` skips the base-language candidate when the exact locale is declared, then falls back to the default after the exact file fails resolution. Users see the default README instead of the available base-language version.
Try the base-language README after an exact-locale candidate fails, before trying the default.
Also at `astrbot/dashboard/services/plugin_service.py:2038`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if README path selection or containment is wrong, the authenticated endpoint could serve unintended Markdown contents from the filesystem; reverting stops future reads but cannot retract content already disclosed.
Blocking findings: astrbot/dashboard/services/plugin_service.py:2037
| if requested in locale_paths: | ||
| matched_locale = requested | ||
| elif "-" in requested: | ||
| base_language = requested.split("-", 1)[0] | ||
| if base_language in locale_paths: | ||
| matched_locale = base_language | ||
| if matched_locale: | ||
| candidates.append((matched_locale, locale_paths[matched_locale])) |
There was a problem hiding this comment.
🟡 Medium · Base-language fallback is skipped
When the requested exact-locale README is declared but missing or invalid, while a base-language README and default README are available, get_plugin_readme() skips the base-language candidate when the exact locale is declared, then falls back to the default after the exact file fails resolution. Users see the default README instead of the available base-language version.
Try the base-language README after an exact-locale candidate fails, before trying the default.
Also at astrbot/dashboard/services/plugin_service.py:2038.
Prompt for AI agents
In `astrbot/dashboard/services/plugin_service.py` at lines 2030-2037:
**Base-language fallback is skipped**
When the requested exact-locale README is declared but missing or invalid, while a base-language README and default README are available, `get_plugin_readme()` skips the base-language candidate when the exact locale is declared, then falls back to the default after the exact file fails resolution. Users see the default README instead of the available base-language version.
Try the base-language README after an exact-locale candidate fails, before trying the default.
Also at `astrbot/dashboard/services/plugin_service.py:2038`.- Guard fetchChangelog with a request id + plugin identity check so a slow changelog response for plugin A can no longer overwrite plugin B's page (same race class that was fixed for the README fetch) - Skip the duplicate README fetch triggered when the plugin-switch watcher resets readmeLocale back to the default - Return the originally declared locale key (e.g. zh-CN) as resolved_locale instead of the normalized lowercase tag, keeping it consistent with available_locales and the documented format - Also catch ValueError when resolving declared README paths so an entry with an embedded NUL byte degrades gracefully instead of failing the request
|
Closing this: the feature has since been implemented upstream, so this PR is no longer needed. Thanks for the reviews! |
Motivation
Closes #10593.
Following the analysis in the issue, the two halves of the request are handled differently:
.astrbot-plugin/i18n/<locale>.jsonandAstrBotConfig. No second language switch was added there, so there is still exactly one source of truth for config strings.get_plugin_readme()always readREADME.md). This PR adds the declaration, the server-side resolution and the dashboard switcher.What changed
Plugin contract
metadata.yamlmay declare README variants:StarMetadata.readmesstores the normalized mapping; invalid entries are dropped at load time so a typo cannot break plugin loading.Resolution
PluginService.get_plugin_readme(plugin_name, locale)resolves in this order:zh-CN),zh-TW→zh),default(orREADME.mdwhen nodefaultis declared).Every declared path must be a relative Markdown file inside the plugin directory:
.., absolute paths, non-Markdown files and symlinks that resolve outside the plugin root are rejected, and a missing variant falls back todefault. Plugins withoutreadmeskeep the exact legacy behavior.The response now also returns
resolved_localeandavailable_locales(additive,contentunchanged).API and dashboard
GET /api/v1/plugins/readme?plugin_id=...&locale=...(the route the dashboard uses) andGET /api/v1/plugins/{plugin_id}/readme?locale=...; the legacy/api/plugin/readmealso forwardslocale. OpenAPI spec updated and the generated client regenerated.PluginDetailPage.vueshows a language select only when the plugin declares more than one README. The default option follows the WebUI locale; picking a variant overrides it for the current detail page. Locale changes are part of the refresh dependencies and responses are guarded by a request id so a slow older response cannot overwrite a newer one.Verification
ruff format --check/ruff checkclean on all touched Python files.python -m py_compileon touched files.pnpm run i18n:checkandpnpm run build(vue-tsc + vite) pass.tests/test_plugin_readme_locale.pycovers: legacy single README, exact locale, language fallback (zh-TW→zh), unknown locale → default, missing variant → default,../escape, absolute path, non-Markdown file, symlink escape, and locale normalization.Notes
readme_urlas before; per-locale marketplace URLs are a separate data-contract change on the marketplace side and are intentionally not included here.Summary by Sourcery
Support localized plugin READMEs with secure locale fallback and a dashboard language switcher.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Review fix (Sourcery finding)
fetchPluginDetail()now carries a request id + plugin identity guard: a slow detail response for plugin A can no longer overwritepluginDetailafter the user switched to plugin B, and the watcher continuation only starts README/changelog fetches when the detail request still belongs to the current selection.