Skip to content

fix: track plugin install source for updates#9037

Merged
Soulter merged 3 commits into
masterfrom
codex/plugin-install-source-policy
Jun 27, 2026
Merged

fix: track plugin install source for updates#9037
Soulter merged 3 commits into
masterfrom
codex/plugin-install-source-policy

Conversation

@Soulter

@Soulter Soulter commented Jun 26, 2026

Copy link
Copy Markdown
Member

Track where plugins were installed from so update checks and update execution use the original plugin market source. Plugins installed from Git URLs, custom URLs, or uploads have update detection and update actions disabled. Legacy plugins without installation source records are treated as installed from the default official plugin market source for update compatibility.

Modifications / 改动点

  • Persist plugin install source records with root directory, install method, registry URL/name, market_plugin_id, repo, download_url, and install time.

  • Validate market installs against the selected registry and use the registry entry's repo/download_url for installation.

  • Resolve plugin updates from the original registry source and disable update/reinstall actions for non-market installs.

  • Resolve missing legacy install-source records through the default official plugin market source and persist a market source record after a successful update.

  • Cache frontend market data by registry URL so update checks for historical sources do not overwrite the current/default market cache.

  • Construct market_plugin_id from market entry author/name when available, and support the documented compatibility exception where the market root key is the plugin metadata name.

  • Document the plugin market JSON format in plugin-market.md, including identity, $meta.schema_version market metadata, required fields, optional download_count, validation, install, update, and source binding rules.

  • Render optional market download counts next to star counts in plugin market cards.

  • Keep market-page installed badges working for older installs by falling back to repo/name for display only.

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

Verification commands passed:

uv run ruff format .
uv run ruff check .
pnpm --dir dashboard exec vue-tsc --noEmit
uv run pytest tests/test_fastapi_v1_dashboard.py -k 'plugin and (install or update or source)'

Note: pnpm prints the existing warning that the package.json pnpm field is ignored; the commands still pass.


Checklist / 检查清单

  • If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。

  • I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txt 和 pyproject.toml 文件相应位置。

  • My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Track and persist plugin installation sources so that update checks and executions use the original marketplace registry, while disabling updates for non-market installs and surfacing this behavior in the dashboard UI.

New Features:

  • Persist plugin installation source metadata for each plugin, including install method, registry, repository, and download URL.
  • Expose install source and update eligibility flags in plugin listing and detail APIs for use by the dashboard.
  • Add per-registry caching of plugin marketplace data in the frontend and store.

Enhancements:

  • Ensure marketplace installs are validated against the selected registry and always use the registry entry’s repository and download URL.
  • Resolve plugin updates from the original marketplace registry instead of relying on the current default source, including for bulk updates.
  • Update the dashboard extension views and cards to compute updates per source, respect backend update eligibility flags, and provide tooltips when updates are disabled.
  • Propagate marketplace metadata through install APIs so backend can correctly associate installed plugins with their source registry.

Tests:

  • Extend dashboard API tests to cover marketplace installs using registry entries and verify that persisted install source data and parameters are correctly handled.

@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. feature:plugin The bug / feature is about AstrBot plugin system. labels Jun 26, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • In update_all_plugins, resolve_market_update_info invokes get_online_plugins for each plugin sequentially, which may re-fetch the same registry data many times; consider grouping plugins by registry_url and reusing marketplace data per registry to reduce network and CPU overhead.
  • The install-source persistence methods (get_plugin_install_sources / save_plugin_install_sources) read and write the entire map for each change; if the number of plugins grows, it may be worth switching to per-plugin updates or a more granular storage pattern to avoid large read–modify–write cycles.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `update_all_plugins`, `resolve_market_update_info` invokes `get_online_plugins` for each plugin sequentially, which may re-fetch the same registry data many times; consider grouping plugins by `registry_url` and reusing marketplace data per registry to reduce network and CPU overhead.
- The install-source persistence methods (`get_plugin_install_sources` / `save_plugin_install_sources`) read and write the entire map for each change; if the number of plugins grows, it may be worth switching to per-plugin updates or a more granular storage pattern to avoid large read–modify–write cycles.

## Individual Comments

### Comment 1
<location path="astrbot/dashboard/services/plugin_service.py" line_range="318" />
<code_context>
             logger.warning(f"获取插件安装时间失败 {plugin.name}: {exc!s}")
             return None

+    @staticmethod
+    def get_market_plugin_id(data: dict[str, Any] | None) -> str:
+        """Return a marketplace plugin ID from the canonical field.
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the tightly coupled install-source–related helpers into a dedicated helper class used by PluginService to keep workflow logic separate from persistence/lookup details.

You can reduce the complexity without changing behavior by grouping the tightly‑coupled “install source” helpers into a dedicated helper object and using it from `PluginService`. This keeps the main class focused on workflows while the source logic lives in one place.

### 1. Extract install‑source logic into a helper

Most of these methods are only about persistence/lookup of install sources:

- `get_plugin_install_sources`
- `save_plugin_install_sources`
- `resolve_plugin_install_source`
- `build_install_source_record`
- `persist_plugin_install_source`
- `remove_plugin_install_source`
- `get_market_plugin_id`
- `normalize_registry_url`
- `resolve_registry_name`
- `repo_identifier_from_url`

You can move them into a small helper, e.g. `PluginInstallSourceStore`, and keep the public surface small:

```python
# new file or inner class: plugin_install_source_store.py

class PluginInstallSourceStore:
    def __init__(self, get_custom_sources):
        self._get_custom_sources = get_custom_sources

    @staticmethod
    def get_market_plugin_id(data: dict[str, Any] | None) -> str:
        if not isinstance(data, dict):
            return ""
        return str(data.get("market_plugin_id") or "").strip()

    @staticmethod
    def normalize_registry_url(value: object) -> str | None:
        text = str(value or "").strip().rstrip("/")
        return text or None

    async def get_all(self) -> dict[str, dict[str, Any]]:
        records = await sp.global_get(PLUGIN_INSTALL_SOURCES_KEY, {})
        if not isinstance(records, dict):
            return {}
        return {str(k): v for k, v in records.items() if isinstance(v, dict)}

    async def save_all(self, records: dict[str, dict[str, Any]]) -> None:
        await sp.global_put(PLUGIN_INSTALL_SOURCES_KEY, records)

    async def resolve_registry_name(self, registry_url: object) -> str:
        normalized_url = self.normalize_registry_url(registry_url)
        if not normalized_url:
            return PLUGIN_DEFAULT_REGISTRY_NAME

        for source in await self._get_custom_sources():
            if not isinstance(source, dict):
                continue
            source_url = self.normalize_registry_url(source.get("url"))
            if source_url != normalized_url:
                continue
            source_name = str(source.get("name") or "").strip()
            return source_name or "Custom"
        return "Custom"

    # small helpers for resolving/building records can also live here...
```

Then `PluginService` uses this object instead of exposing many static helpers:

```python
class PluginService:
    def __init__(self, ...):
        ...
        self.install_sources = PluginInstallSourceStore(self.get_custom_sources)

    async def list_plugins_from_dashboard(self, ...):
        install_sources = await self.install_sources.get_all()
        ...
        install_source = self.install_sources.resolve_for_plugin(plugin, install_sources)
        ...

    async def install_plugin(self, data: object) -> tuple[dict, str]:
        ...
        plugin_info = await self.plugin_manager.install_plugin(...)
        await self.install_sources.persist_after_install(
            plugin_info=plugin_info,
            payload=payload,
            fallback_method="github" if self.repo_identifier_from_url(repo_url) else "url",
            repo_url=repo_url,
            download_url=download_url,
        )
        ...

    async def uninstall_plugin(self, data: object) -> tuple[None, str]:
        ...
        await self.install_sources.remove(root_dir_name)
```

This keeps all the install‑source rules the same, but `PluginService` now reads as:

- “get install sources”
- “resolve install source”
- “persist/remove install source”

instead of dozens of low‑level methods mixed into the main class.

You could apply the same idea later to the marketplace matching (`iter_market_plugin_entries`, `resolve_market_plugin_entry`, `resolve_market_plugin_entry_by_id`) by introducing a small `MarketIndex` or `MarketSourceResolver`, but even just extracting the install‑source helpers will noticeably reduce the cognitive load in `PluginService`.
</issue_to_address>

### Comment 2
<location path="dashboard/src/views/extension/useExtensionPage.js" line_range="683" />
<code_context>
-      // 使用 marketplace_name 进行市场匹配(后端已统一为减号格式)
-      const normalizedExtensionName = normalizeStr(extension.marketplace_name);
-      const onlinePluginByName = onlinePluginsNameMap.get(normalizedExtensionName);
+  const checkUpdate = async () => {
+    const data = Array.isArray(extension_data?.data) ? extension_data.data : [];
+    const sourcePlugins = new Map();
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the shared plugin-matching and state-update logic into reusable helpers so `checkUpdate` and `checkAlreadyInstalled` only orchestrate the flow.

You can keep all the new behavior but reduce complexity by extracting the matching logic and state‑mutation logic you’ve inlined into `checkUpdate` / `checkAlreadyInstalled`.

### 1. Centralize matching rules

Right now, you’re re‑implementing the same matching rules in both `checkUpdate` and `checkAlreadyInstalled` (identifier → repo → name). You already have `buildMarketPluginLookup`; you can add a single helper that encapsulates the matching strategy and reuse it everywhere.

```js
const matchMarketPluginForInstalled = (
  extension,
  lookup,
  { registryUrl } = {},
) => {
  const source = extension.install_source || {};
  const isCurrentMarketInstall =
    source.install_method === "market" &&
    normalizeRegistryUrl(source.registry_url) === normalizeRegistryUrl(registryUrl);

  const sourceIdentifier = isCurrentMarketInstall
    ? String(source.market_plugin_id || "").trim()
    : "";

  const sourceRepo = normalizeInstallUrl(source.repo || extension.repo).toLowerCase();
  const normalizedName = normalizeStr(extension.marketplace_name || extension.name);

  return (
    (sourceIdentifier && lookup.byIdentifier.get(sourceIdentifier)) ||
    (sourceRepo && lookup.byRepo.get(sourceRepo)) ||
    lookup.byName.get(normalizedName) ||
    null
  );
};
```

Then `checkUpdate` becomes simpler orchestration:

```js
const resetUpdateState = (ext) => {
  ext.online_version = "";
  ext.has_update = false;
  ext.update_market_plugin = null;
};

const applyMarketMatch = (ext, plugin) => {
  if (!plugin) return;
  ext.update_market_plugin = plugin;
  ext.online_version = plugin.version;
  ext.has_update =
    ext.version !== plugin.version &&
    plugin.version !== tm("status.unknown");
};

const groupExtensionsByRegistry = (extensions) => {
  const groups = new Map();
  extensions.forEach((ext) => {
    resetUpdateState(ext);

    const source = ext.install_source;
    if (!ext.updates_enabled || !source || source.install_method !== "market") {
      return;
    }

    const registryUrl = normalizeRegistryUrl(source.registry_url);
    if (!groups.has(registryUrl)) groups.set(registryUrl, []);
    groups.get(registryUrl).push(ext);
  });
  return groups;
};

const checkUpdate = async () => {
  const data = Array.isArray(extension_data?.data) ? extension_data.data : [];
  const sourceGroups = groupExtensionsByRegistry(data);

  await Promise.all(
    [...sourceGroups.entries()].map(async ([registryUrl, extensions]) => {
      let marketPlugins = [];
      try {
        marketPlugins =
          normalizeRegistryUrl(registryUrl) === normalizeRegistryUrl(selectedSource.value)
            ? pluginMarketData.value
            : await commonStore.getPluginCollections(false, registryUrl || null);
      } catch (error) {
        console.warn("Failed to load plugin source for update check:", error);
        return;
      }

      const lookup = buildMarketPluginLookup(marketPlugins);
      extensions.forEach((ext) => {
        const plugin = matchMarketPluginForInstalled(ext, lookup, { registryUrl });
        applyMarketMatch(ext, plugin);
      });
    }),
  );
};
```

### 2. Reuse the same matching in `checkAlreadyInstalled`

`checkAlreadyInstalled` can also use the same `buildMarketPluginLookup` + `matchMarketPluginForInstalled` instead of rebuilding its own identifier/repo/name maps:

```js
const checkAlreadyInstalled = () => {
  const data = Array.isArray(extension_data?.data) ? extension_data.data : [];
  const currentRegistryUrl = normalizeRegistryUrl(selectedSource.value);

  const lookup = buildMarketPluginLookup(pluginMarketData.value);

  pluginMarketData.value.forEach((plugin) => {
    const installed = data.find((ext) =>
      matchMarketPluginForInstalled(ext, lookup, { registryUrl: currentRegistryUrl }) === plugin,
    );

    if (installed) {
      // existing metadata backfill logic, unchanged
      if (
        (!Array.isArray(plugin.support_platforms) || plugin.support_platforms.length === 0) &&
        Array.isArray(installed.support_platforms)
      ) {
        plugin.support_platforms = installed.support_platforms;
      }
      // ...
    }
  });
};
```

This keeps:

- all your multi‑source behavior,
- all existing flags (`update_market_plugin`, `online_version`, `has_update`, etc.),

but puts the matching rules and state changes in one place, making `checkUpdate` and `checkAlreadyInstalled` much easier to reason about and safer to evolve.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

logger.warning(f"获取插件安装时间失败 {plugin.name}: {exc!s}")
return None

@staticmethod

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the tightly coupled install-source–related helpers into a dedicated helper class used by PluginService to keep workflow logic separate from persistence/lookup details.

You can reduce the complexity without changing behavior by grouping the tightly‑coupled “install source” helpers into a dedicated helper object and using it from PluginService. This keeps the main class focused on workflows while the source logic lives in one place.

1. Extract install‑source logic into a helper

Most of these methods are only about persistence/lookup of install sources:

  • get_plugin_install_sources
  • save_plugin_install_sources
  • resolve_plugin_install_source
  • build_install_source_record
  • persist_plugin_install_source
  • remove_plugin_install_source
  • get_market_plugin_id
  • normalize_registry_url
  • resolve_registry_name
  • repo_identifier_from_url

You can move them into a small helper, e.g. PluginInstallSourceStore, and keep the public surface small:

# new file or inner class: plugin_install_source_store.py

class PluginInstallSourceStore:
    def __init__(self, get_custom_sources):
        self._get_custom_sources = get_custom_sources

    @staticmethod
    def get_market_plugin_id(data: dict[str, Any] | None) -> str:
        if not isinstance(data, dict):
            return ""
        return str(data.get("market_plugin_id") or "").strip()

    @staticmethod
    def normalize_registry_url(value: object) -> str | None:
        text = str(value or "").strip().rstrip("/")
        return text or None

    async def get_all(self) -> dict[str, dict[str, Any]]:
        records = await sp.global_get(PLUGIN_INSTALL_SOURCES_KEY, {})
        if not isinstance(records, dict):
            return {}
        return {str(k): v for k, v in records.items() if isinstance(v, dict)}

    async def save_all(self, records: dict[str, dict[str, Any]]) -> None:
        await sp.global_put(PLUGIN_INSTALL_SOURCES_KEY, records)

    async def resolve_registry_name(self, registry_url: object) -> str:
        normalized_url = self.normalize_registry_url(registry_url)
        if not normalized_url:
            return PLUGIN_DEFAULT_REGISTRY_NAME

        for source in await self._get_custom_sources():
            if not isinstance(source, dict):
                continue
            source_url = self.normalize_registry_url(source.get("url"))
            if source_url != normalized_url:
                continue
            source_name = str(source.get("name") or "").strip()
            return source_name or "Custom"
        return "Custom"

    # small helpers for resolving/building records can also live here...

Then PluginService uses this object instead of exposing many static helpers:

class PluginService:
    def __init__(self, ...):
        ...
        self.install_sources = PluginInstallSourceStore(self.get_custom_sources)

    async def list_plugins_from_dashboard(self, ...):
        install_sources = await self.install_sources.get_all()
        ...
        install_source = self.install_sources.resolve_for_plugin(plugin, install_sources)
        ...

    async def install_plugin(self, data: object) -> tuple[dict, str]:
        ...
        plugin_info = await self.plugin_manager.install_plugin(...)
        await self.install_sources.persist_after_install(
            plugin_info=plugin_info,
            payload=payload,
            fallback_method="github" if self.repo_identifier_from_url(repo_url) else "url",
            repo_url=repo_url,
            download_url=download_url,
        )
        ...

    async def uninstall_plugin(self, data: object) -> tuple[None, str]:
        ...
        await self.install_sources.remove(root_dir_name)

This keeps all the install‑source rules the same, but PluginService now reads as:

  • “get install sources”
  • “resolve install source”
  • “persist/remove install source”

instead of dozens of low‑level methods mixed into the main class.

You could apply the same idea later to the marketplace matching (iter_market_plugin_entries, resolve_market_plugin_entry, resolve_market_plugin_entry_by_id) by introducing a small MarketIndex or MarketSourceResolver, but even just extracting the install‑source helpers will noticeably reduce the cognitive load in PluginService.

// 使用 marketplace_name 进行市场匹配(后端已统一为减号格式)
const normalizedExtensionName = normalizeStr(extension.marketplace_name);
const onlinePluginByName = onlinePluginsNameMap.get(normalizedExtensionName);
const checkUpdate = async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (complexity): Consider extracting the shared plugin-matching and state-update logic into reusable helpers so checkUpdate and checkAlreadyInstalled only orchestrate the flow.

You can keep all the new behavior but reduce complexity by extracting the matching logic and state‑mutation logic you’ve inlined into checkUpdate / checkAlreadyInstalled.

1. Centralize matching rules

Right now, you’re re‑implementing the same matching rules in both checkUpdate and checkAlreadyInstalled (identifier → repo → name). You already have buildMarketPluginLookup; you can add a single helper that encapsulates the matching strategy and reuse it everywhere.

const matchMarketPluginForInstalled = (
  extension,
  lookup,
  { registryUrl } = {},
) => {
  const source = extension.install_source || {};
  const isCurrentMarketInstall =
    source.install_method === "market" &&
    normalizeRegistryUrl(source.registry_url) === normalizeRegistryUrl(registryUrl);

  const sourceIdentifier = isCurrentMarketInstall
    ? String(source.market_plugin_id || "").trim()
    : "";

  const sourceRepo = normalizeInstallUrl(source.repo || extension.repo).toLowerCase();
  const normalizedName = normalizeStr(extension.marketplace_name || extension.name);

  return (
    (sourceIdentifier && lookup.byIdentifier.get(sourceIdentifier)) ||
    (sourceRepo && lookup.byRepo.get(sourceRepo)) ||
    lookup.byName.get(normalizedName) ||
    null
  );
};

Then checkUpdate becomes simpler orchestration:

const resetUpdateState = (ext) => {
  ext.online_version = "";
  ext.has_update = false;
  ext.update_market_plugin = null;
};

const applyMarketMatch = (ext, plugin) => {
  if (!plugin) return;
  ext.update_market_plugin = plugin;
  ext.online_version = plugin.version;
  ext.has_update =
    ext.version !== plugin.version &&
    plugin.version !== tm("status.unknown");
};

const groupExtensionsByRegistry = (extensions) => {
  const groups = new Map();
  extensions.forEach((ext) => {
    resetUpdateState(ext);

    const source = ext.install_source;
    if (!ext.updates_enabled || !source || source.install_method !== "market") {
      return;
    }

    const registryUrl = normalizeRegistryUrl(source.registry_url);
    if (!groups.has(registryUrl)) groups.set(registryUrl, []);
    groups.get(registryUrl).push(ext);
  });
  return groups;
};

const checkUpdate = async () => {
  const data = Array.isArray(extension_data?.data) ? extension_data.data : [];
  const sourceGroups = groupExtensionsByRegistry(data);

  await Promise.all(
    [...sourceGroups.entries()].map(async ([registryUrl, extensions]) => {
      let marketPlugins = [];
      try {
        marketPlugins =
          normalizeRegistryUrl(registryUrl) === normalizeRegistryUrl(selectedSource.value)
            ? pluginMarketData.value
            : await commonStore.getPluginCollections(false, registryUrl || null);
      } catch (error) {
        console.warn("Failed to load plugin source for update check:", error);
        return;
      }

      const lookup = buildMarketPluginLookup(marketPlugins);
      extensions.forEach((ext) => {
        const plugin = matchMarketPluginForInstalled(ext, lookup, { registryUrl });
        applyMarketMatch(ext, plugin);
      });
    }),
  );
};

2. Reuse the same matching in checkAlreadyInstalled

checkAlreadyInstalled can also use the same buildMarketPluginLookup + matchMarketPluginForInstalled instead of rebuilding its own identifier/repo/name maps:

const checkAlreadyInstalled = () => {
  const data = Array.isArray(extension_data?.data) ? extension_data.data : [];
  const currentRegistryUrl = normalizeRegistryUrl(selectedSource.value);

  const lookup = buildMarketPluginLookup(pluginMarketData.value);

  pluginMarketData.value.forEach((plugin) => {
    const installed = data.find((ext) =>
      matchMarketPluginForInstalled(ext, lookup, { registryUrl: currentRegistryUrl }) === plugin,
    );

    if (installed) {
      // existing metadata backfill logic, unchanged
      if (
        (!Array.isArray(plugin.support_platforms) || plugin.support_platforms.length === 0) &&
        Array.isArray(installed.support_platforms)
      ) {
        plugin.support_platforms = installed.support_platforms;
      }
      // ...
    }
  });
};

This keeps:

  • all your multi‑source behavior,
  • all existing flags (update_market_plugin, online_version, has_update, etc.),

but puts the matching rules and state changes in one place, making checkUpdate and checkAlreadyInstalled much easier to reason about and safer to evolve.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to track, persist, and display plugin installation sources, disabling updates for plugins not installed via the marketplace. The feedback highlights a critical race condition in persist_plugin_install_source where concurrent modifications could be lost during an asynchronous call, as well as opportunities to improve repository URL parsing and matching robustness by handling URLs without schemes and comparing parsed repository identifiers.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +526 to +535
records = await self.get_plugin_install_sources()
records[plugin.root_dir_name] = self.build_install_source_record(
plugin,
payload,
fallback_method=fallback_method,
repo_url=repo_url,
download_url=download_url,
registry_name=await self.resolve_registry_name(payload.get("registry_url")),
)
await self.save_plugin_install_sources(records)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a race condition here. The asynchronous call await self.resolve_registry_name(...) is executed after fetching the records dictionary but before saving it back. During this await, other concurrent tasks (such as other plugin installations or updates) could modify the global plugin install sources, and those changes would be overwritten and lost when save_plugin_install_sources is called. Resolve the registry name before fetching the records to keep the read-and-write block atomic.

Suggested change
records = await self.get_plugin_install_sources()
records[plugin.root_dir_name] = self.build_install_source_record(
plugin,
payload,
fallback_method=fallback_method,
repo_url=repo_url,
download_url=download_url,
registry_name=await self.resolve_registry_name(payload.get("registry_url")),
)
await self.save_plugin_install_sources(records)
registry_name = await self.resolve_registry_name(payload.get("registry_url"))
records = await self.get_plugin_install_sources()
records[plugin.root_dir_name] = self.build_install_source_record(
plugin,
payload,
fallback_method=fallback_method,
repo_url=repo_url,
download_url=download_url,
registry_name=registry_name,
)
await self.save_plugin_install_sources(records)
References
  1. In a single-threaded asyncio event loop, synchronous functions (code blocks without 'await') are executed atomically and will not be interrupted by other coroutines. Therefore, they are safe from race conditions when modifying shared state within that block.

Comment on lines +441 to +444
text = str(repo_url or "").strip().rstrip("/")
if not text:
return ""
parsed = urlparse(text)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the repository URL does not contain a scheme (e.g., github.com/owner/repo), urlparse will parse the entire string as a path, resulting in an empty netloc and causing the identification to fail. Prepend a default scheme if none is present to ensure robust parsing.

Suggested change
text = str(repo_url or "").strip().rstrip("/")
if not text:
return ""
parsed = urlparse(text)
text = str(repo_url or "").strip().rstrip("/")
if not text:
return ""
if not text.startswith(("http://", "https://")):
text = "https://" + text
parsed = urlparse(text)

Comment on lines +1130 to +1143
record_identifier = self.get_market_plugin_id(record)
record_repo_identifier = self.repo_identifier_from_url(record.get("repo"))
record_repo = str(record.get("repo") or "").strip().rstrip("/").lower()

for _, plugin in self.iter_market_plugin_entries(market_data):
plugin_identifier = self.get_market_plugin_id(plugin)
if record_identifier and plugin_identifier == record_identifier:
return plugin
if record_repo_identifier and plugin_identifier == record_repo_identifier:
return plugin
plugin_repo = str(plugin.get("repo") or "").strip().rstrip("/").lower()
if record_repo and plugin_repo == record_repo:
return plugin
return None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

When matching the marketplace entry, comparing raw repository URLs directly (plugin_repo == record_repo) can fail if one URL has a .git suffix or a different subdomain (e.g., www.github.com vs github.com). Comparing their parsed repository identifiers using repo_identifier_from_url makes the matching process much more robust.

        record_identifier = self.get_market_plugin_id(record)
        record_repo_identifier = self.repo_identifier_from_url(record.get("repo"))
        record_repo = str(record.get("repo") or "").strip().rstrip("/").lower()

        for _, plugin in self.iter_market_plugin_entries(market_data):
            plugin_identifier = self.get_market_plugin_id(plugin)
            if record_identifier and plugin_identifier == record_identifier:
                return plugin
            if record_repo_identifier and plugin_identifier == record_repo_identifier:
                return plugin
            plugin_repo_identifier = self.repo_identifier_from_url(plugin.get("repo"))
            if record_repo_identifier and plugin_repo_identifier and record_repo_identifier == plugin_repo_identifier:
                return plugin
            plugin_repo = str(plugin.get("repo") or "").strip().rstrip("/").lower()
            if record_repo and plugin_repo == record_repo:
                return plugin
        return None

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
astrbot-docs de8a520 Commit Preview URL

Branch Preview URL
Jun 27 2026, 06:55 AM

@Soulter Soulter force-pushed the codex/plugin-install-source-policy branch from 126fe1e to 57f9a08 Compare June 27, 2026 04:09
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jun 27, 2026
Comment on lines +1071 to +1074
return await _run_service(
service.bind_plugin_market_source({"name": plugin_id, **body}),
log_label="/api/plugin/source",
)
@Soulter Soulter force-pushed the codex/plugin-install-source-policy branch 8 times, most recently from 594750b to ac39c5d Compare June 27, 2026 05:26
@Soulter Soulter force-pushed the codex/plugin-install-source-policy branch 4 times, most recently from c6e6fb7 to 3d0a513 Compare June 27, 2026 06:32
@Soulter Soulter force-pushed the codex/plugin-install-source-policy branch from 3d0a513 to 231cc26 Compare June 27, 2026 06:40
@Soulter Soulter merged commit 0ef790d into master Jun 27, 2026
22 checks passed
@Soulter Soulter deleted the codex/plugin-install-source-policy branch June 27, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature:plugin The bug / feature is about AstrBot plugin system. size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants