Plugin identity: explicit group + id, with an install-time conflict gate - #153
Conversation
… themselves
A plugin installed via a market gets a registry id prefixed with
<marketId>- (e.g. "official-container-manager") to avoid colliding with
another market publishing the same plugin id (installer.ts's
installedPluginManifests()). But the plugin's own bundled code has no idea
about that prefix and always calls usePluginSdkFor/getPluginSdk with its
bare, self-authored id (e.g. usePluginSdkFor('container-manager')) — it
can't know at build time which market, if any, it'll be installed under.
usePluginSdkFor's context match compared against the namespaced `id`, so
it never matched for any market install, falling through to
getPlugin(bareId) — which also only ever indexed PLUGIN_MAP by the
namespaced id — and threw "Không có plugin ... trong registry" even
though the plugin was installed and actively rendering. getPluginSdk
(used by plugins for module-scope stores, e.g. Kafka's/RabbitMQ's
consumer stores that must survive route changes) had the exact same
bug, unconditionally.
Fix: track the original, un-prefixed id separately as `baseId` on
PluginManifest/PluginSdk (installer.ts sets it whenever it builds a
market-prefixed registry id), match usePluginSdkFor's context check
against `baseId` instead of `id`, and have getPlugin() fall back to a
baseId scan when the exact (possibly-prefixed) key misses. The `id` field
itself is untouched everywhere it actually needs to stay namespaced
(storage keys, audit log, PLUGIN_MAP routing) — only the plugin's
self-lookup path changes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #153 +/- ##
==========================================
+ Coverage 42.03% 42.07% +0.04%
==========================================
Files 299 299
Lines 19848 19864 +16
Branches 4904 4909 +5
==========================================
+ Hits 8343 8358 +15
Misses 10508 10508
- Partials 997 998 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one coverage gap inline, plus a limitation worth tracking.
Reviewed changes
baseIdadded to manifests/SDK —PluginManifest.baseIdcarries the un-prefixed plugin id;createPluginSdkdefaults it toid, so compile-time and non-market installs are unchanged.installer.tssetsbaseId— market-prefixed records keep the original bare id inbaseIdwhileid/route/storage/audit stay namespaced.getPluginfalls back to abaseIdscan — a bare-id lookup now resolves a market install whose registry id is prefixed.usePluginSdkFormatches context bybaseId— in-tree plugin code gets its own SDK instead of falling through to the throw.- Tests — registry lookup by prefixed id and by bare
baseId;baseIddefaulting and override in the SDK.
Verified tsc --noEmit and the targeted platform suite (registry.installer, sdk, installer) — all green.
ℹ️ Module-scope getPluginSdk(bareId) cannot disambiguate two markets shipping the same id
The in-tree path (usePluginSdkFor) is now correct because the context carries the exact install's baseId. The module-scope path (getPluginSdk) has no such context: with two markets shipping the same bare id, getPlugin(bareId) returns whichever install registered first, so the second install's module-scope stores silently get the first install's SDK — wrong storage namespace, audit id, and permission set. This is the exact two-market collision the <marketId>- prefix was built to support, and the fallback is what newly allows a wrong-but-successful resolution instead of a throw. No clean fix without bundle-scoped identity injection, so it may well be an accepted limitation — but worth an explicit decision rather than a comment.
Technical details
# Module-scope plugin identity is ambiguous across markets
## Affected sites
- `src/platform/registry.ts:145` — `records.find((p) => p.baseId === id)` returns the first registration sharing `baseId`; no `baseId` clash check exists in `registerManifest`.
- `src/platform/usePluginSdkFor.ts:19` — `getPluginSdk` caches by the bare `pluginId`, so the second install never gets a distinct SDK instance.
- `src/components/tools/mockserver/requestLogStore.ts:34`, `src/components/tools/apiclient/request.ts:23` — module-scope consumers that call `getPluginSdk(bareId)`.
## Required outcome
- Decide whether duplicate-`baseId` installs across markets are acceptable as first-wins, or whether module-scope identity needs to be bundle-scoped (host injects the resolved registry id before `import()` of the bundle) so each install resolves itself.
## Open questions for the human
- Is installing the same plugin id from two different markets a supported scenario worth solving now, or an accepted edge to document/track?Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
Both existing marketId tests checked the prefixed id/route but never baseId itself — deleting installer.ts's `baseId: record.manifest.id` line kept every test green while reintroducing the exact bug this PR fixes (usePluginSdkFor/getPluginSdk throwing "not in registry" for any market-installed plugin). Confirmed by temporarily removing that line locally: these two new assertions fail, everything else still passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
|
Re: the module-scope Before this PR, installing the same plugin id from two markets meant BOTH installs' Not blocking merge; happy to open a follow-up if this becomes a real scenario. Generated by Claude Code |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Coverage gap closed —
installer.test.tsnow assertsbaseIdin both the non-market and market-prefixed cases, so deletinginstaller.ts'sbaseId: record.manifest.idline fails these assertions instead of silently reintroducing the original "not in registry" bug.
The remaining body note from the prior review (module-scope getPluginSdk(bareId) is first-wins when two markets ship the same bare id) is unchanged and still stands as an accepted-limitation decision for the author.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
…y it Follow-up to the market-prefix fix: a plugin installed from a bare URL (no marketId, keeps its unprefixed id) and the SAME plugin installed again from a market (id gets prefixed <marketId>-) don't collide on id/route/order at all — those checks all compare different strings between the two records, so both registered successfully. That's worse than it sounds: getPluginSdk(bareId) (module-scope, no React context — used by Kafka's/RabbitMQ's consumer stores) looks up PLUGIN_MAP by the exact string first, before ever trying the baseId fallback. With both plugins registered, that exact lookup hits the bare-URL install directly regardless of which bundle's code is actually calling — the market-installed instance's module-scope calls would silently receive the URL install's SDK: wrong storage namespace, wrong audit id, wrong permission set. Two independent installs would leak into each other's persisted state. Added a seenBaseIds guard alongside the existing seenIds/seenRoutes/ seenOrders in registerManifest — a second registration sharing an effective baseId (m.baseId ?? m.id) is now rejected into PLUGIN_ERRORS like any other collision, instead of silently coexisting and letting getPlugin()'s baseId fallback (and getPluginSdk's cache) misattribute identity. Also simplified getPlugin()'s doc comment: with duplicates now rejected at registration, its baseId scan can only ever match one record, no more first-wins ambiguity to describe. New test reproduces the exact scenario (URL install + market install of the same plugin) and confirms it now fails closed instead of silently misidentifying; confirmed by temporarily reverting the guard, which makes the test fail for the right reason. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Duplicate
baseIdnow rejected at registration —registerManifestkeys a newseenBaseIdsmap bym.baseId ?? m.idand refuses the second registration sharing a bare id, sogetPlugin(baseId)/getPluginSdk(baseId)can only ever resolve one record. This closes the module-scope ambiguity raised in the prior review rather than leaving it as an accepted limitation. - Regression test added —
registry.installer.test.tscovers a URL-installed copy plus a market-installed copy of the same bare id (all ofid/route/orderdistinct) and asserts the second is rejected with aPLUGIN_ERRORSentry. It fails without theseenBaseIdsclause, so it is real coverage. - Doc comments updated —
getPlugin's comment no longer describes duplicate-baseIdambiguity as a known limitation, matching the new invariant.
Verified tsc --noEmit and the platform suite (registry.installer, registry, migration, sdk, installer) — all green.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
Supersedes this PR's earlier baseId field with what it was really approximating: every plugin now has an `id` (self-authored, what the plugin's own bundle calls itself via usePluginSdkFor/getPluginSdk) and a `group` (where it was installed from — "core" for the 26 compile-time plugins, "url" for a bare-URL install, or the marketId for a market install). Metadata only: storage/secrets/audit keys stay keyed by bare `id` exactly as before, so no migration for anyone's existing data. `id` still ends up unique system-wide in practice, not just per group: usePluginSdkFor/getPluginSdk are called by a plugin's own bundle code with just the bare id string, no group — module-scope calls (Kafka's/ RabbitMQ's background consumer stores) have no React context to resolve group from. Letting the same id genuinely coexist under two groups would make that lookup ambiguous and silently misattribute storage/audit/ permissions to the wrong plugin. So installer.ts now enforces it at the one point that matters — install time, not registry time: `assertNoConflictingInstall(id, marketId)` checks the already-installed list for the same bare id under a DIFFERENT group and throws a clear "uninstall the old one first" error before Rust ever writes to disk, called from both install call sites (ExtensionInstallDialog, SettingsExtensionInstaller) using the manifest they've already fetched for the confirmation screen — not fetched a second time inside installArtifact itself, which would have doubled a network round-trip AND changed the call count contract every existing install-flow test asserted on. registry.ts drops the seenBaseIds/getPlugin-fallback machinery entirely now that id is flat again: registerManifest just gets group defaulted to 'core' for the 26 compile-time plugins that don't declare it (installer.ts always declares it explicitly for runtime installs), and getPlugin(id) is back to a plain PLUGIN_MAP.get(id) — no ambiguity left to fall back on. Route generation for installed plugins is now uniform for both URL and market installs: /installed/<group>/<id>, replacing the old asymmetric "market gets a generated route, URL install keeps its own" split. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
DianaSensei
changed the title
Codecov flagged a partial branch: the new `preview?.kind === 'plugin'` check gating assertNoConflictingInstall had only its true side (plugin install) exercised by any test — no test ever clicked Install for a kind=service preview. Extended the existing service-preview test to also install, asserting it skips straight to artifact_installer_install with no list call in between (assertNoConflictingInstall only applies to plugins). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
There was a problem hiding this comment.
ℹ️ No critical issues — the redesign is sound; a couple of minor suggestions inline plus stale docs.
Reviewed changes
baseIdreplaced bygroup+ bareid—PluginManifest.group?,PluginSdk.group, and a requiredPluginRecord.group;idis the bare, self-authored id again and remains the storage/secrets/audit namespace key.installer.tsemits bareid+group+ a uniform route —installedPluginManifests()now generates/installed/<group>/<id>for every runtime install and no longer prefixes the registry id.- New install-time conflict gate —
assertNoConflictingInstall(id, marketId)rejects installing a bareidalready installed under a different group, called from both install UIs beforeinstallArtifact. registry.tssimplified — flatseenIdsand plainPLUGIN_MAP.get(id); theseenBaseIdsfallback machinery is removed.- Tests — conflict-gate cases, bare-id market registration + duplicate-id rejection,
groupdefaulting, and updated id/route assertions.
Verified tsc --noEmit and the targeted platform suite (installer, registry.installer, sdk, registry) — 79/79 green.
ℹ️ Nitpicks
docs/plugin-sdk/02-manifest.md's[^market-id]footnote (lines 45-55) still documents the removed behavior — two markets with the same id coexisting, registry id<marketId>-<id>, route/installed/<marketId>-<id>, and bare-URL installs keeping their own route. All three are now false; since this is the plugin-author contract, it should be updated.src/lib/market.ts:95-100justifies the kebab-case market-id rule by "installedPluginManifests()... ghép ... (${marketId}-${pluginId})" — that join no longer exists.- Bare-URL installs now use the generated
/installed/url/<id>route instead of theroutethey declare in their remote manifest, so an already-installed plugin's URL changes with no redirect. Probably fine, but worth a release note.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
| // tố market, xem `PluginManifest.group`. `installArtifact()` chặn cài | ||
| // nếu id này đã bị một group khác chiếm, nên tại registry nó vẫn | ||
| // nhất quán duy nhất toàn app dù không ép kiểu ở đây. | ||
| id: record.manifest.id, |
There was a problem hiding this comment.
Making id bare changes the storage/secrets namespace for existing market installs: on main a market install registered as <marketId>-<id>, and storageKey/vaultKey/audit all key off that registry id, so its data lives under devtool:official-<id>:… / official-<id>/…. After this change the plugin reads devtool:<id>:… / <id>/… with no migration, so the PR body's "no data migration for anyone's existing installs" isn't accurate. Impact is probably small (market plugins couldn't resolve an SDK before this fix, so they likely never wrote), but the claim should be corrected or a one-time move added.
Technical details
# Market-install storage namespace changes from `<marketId>-<id>` to `<id>`
## Affected sites
- `src/platform/installer.ts:538` — `id: record.manifest.id` (was `id: registryId`, `registryId = marketId ? `${marketId}-${id}` : id`).
- `src/platform/sdk.ts:228` — `storageKey(id, key)` → `devtool:<id>:<key>`.
- `src/platform/secrets.ts:51` — `vaultKey(id, key)` → `<id>/<key>`.
- `src/platform/audit.ts` — in-RAM only, so audit history is unaffected.
## Required outcome
- Either add a one-time migration (the `usePluginState` `legacyKey` mechanism is the house pattern) from `devtool:<marketId>-<id>:*` / `<marketId>-<id>/*` to the bare-id namespace, or correct the claim that no migration is needed.
## Open questions for the human
- Are there any released market installs with persisted settings/secrets worth migrating, or is this a no-op in practice because those plugins were broken?There was a problem hiding this comment.
Correct that the namespace string itself changes — my claim was imprecise. Answering the actual question though: no data to migrate. usePluginSdkFor(id) threw before any market-installed plugin's component could reach a single sdk.storage/sdk.secrets call — that's the exact bug this whole PR chain fixes. So the old <marketId>-<id> namespace was never written to for any market install; there's nothing under it to move. Documented this reasoning inline at installer.ts's id: record.manifest.id line in c7946d2 rather than leaving it only in commit history, per your point that the claim should be justified, not just asserted.
Generated by Claude Code
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
kind=serviceinstall path covered —SettingsExtensionInstaller.test.tsxnow installs from a service preview and asserts the flow reachesartifact_installer_installwith no precedinglistInstalledPluginscall, locking in thatassertNoConflictingInstallruns only forkind: "plugin".
The new test can fail for the right reason: widening the preview?.kind === 'plugin' guard to all kinds would insert an artifact_installer_list call that consumes the mocked install response, so the install assertion would break.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
Pullfrog review, PR #153: 1. assertNoConflictingInstall() only scanned listInstalledPlugins() (the on-disk index) — it had no visibility into the 26 compile-time plugins, which never appear there. Installing a plugin whose id collides with a built-in (e.g. "json") passed the gate, got written to disk, and was only THEN silently rejected by registerManifest's flat seenIds on the next launch — exactly the deferred-to-restart PLUGIN_ERRORS outcome this gate exists to front-load. Fixed by adding an optional `registeredGroup` parameter: the two UI call sites (ExtensionInstallDialog, SettingsExtensionInstaller) now pass `getPlugin(id)?.group` — already exported from '@/platform', no new registry->installer import needed (installer.ts can't import registry.ts; registry.ts already imports installer.ts). A `group: 'core'` conflict gets its own error message ("trùng id với một tool có sẵn") since "uninstall the old one" doesn't apply to a built-in tool. 2. Corrected the PR's "no data migration needed" claim into an actual justification rather than an assertion: storage/secrets namespacing for a market install did change from the old `<marketId>-<id>` prefix to a bare `id`, but no market-installed plugin ever successfully read/wrote sdk.storage/sdk.secrets before this PR — usePluginSdkFor(id) threw before the component could reach any such call. Documented inline at the exact line the namespace is set, instead of just in commit history. 3. Updated the two doc/comment sites Pullfrog found still describing the removed <marketId>-<id> registry-id scheme: 02-manifest.md's plugin-author-facing footnote, and market.ts's kebab-case rationale comment. New tests cover the registeredGroup parameter (core collision throws its own message, matching group is a no-op); confirmed by temporarily reverting the registeredGroup branch, which makes the new test fail for the right reason. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
|
The This is a failure in GitHub's own Copilot backend configuration ( Generated by Claude Code |
Codecov flagged a gap: the two new registeredGroup tests only covered the "core" collision message and the "matches, falls through to disk scan" case — never the plain "already installed under a different group" throw reached via the registeredGroup parameter itself (as opposed to the pre-existing disk-scan path that produces the same message text). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
There was a problem hiding this comment.
ℹ️ No critical issues — one minor suggestion inline.
Reviewed changes
registeredGroupadded to the install-time conflict gate —assertNoConflictingInstallnow accepts the registry group of an existing record and rejects acoreid collision with its own "tool có sẵn" message; both install UIs passgetPlugin(id)?.group, closing the built-in-id blind spot from the prior review.- Docs and comments realigned to the
group+ bare-idmodel — the[^market-id]footnote indocs/plugin-sdk/02-manifest.mdand the kebab-case rationale insrc/lib/market.tsno longer describe the removed<marketId>-<id>scheme; the storage-namespace change is now justified inline atinstaller.ts. - Tests — new
registeredGroupcases: acorecollision throws the built-in-specific message without touchinglistInstalledPlugins, and a matching group falls through to the on-disk scan.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in the latest commit — test-only coverage.
Reviewed changes
- Non-core
registeredGroupbranch covered —installer.test.tsnow asserts that a registry record under a different non-core group throws the "gỡ bản đó" message beforelistInstalledPluginsis called, mirroring the existingcore-collision case and failing if that branch is removed.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
assertNoConflictingInstall's early registeredGroup check threw for ANY registeredGroup !== newGroup, not just 'core'. But the in-memory registry (PLUGIN_MAP) only loads once at app bootstrap and never updates when a plugin is installed/uninstalled during a running session. So a user who uninstalls a market plugin and immediately reinstalls the same id from a different source (without restarting) would be incorrectly blocked by a stale registeredGroup — even though the disk scan below it would correctly show no conflict. Fix: restrict the early-throw to registeredGroup === 'core' only, since compile-time plugins never disappear from the registry mid-session and are safe to short-circuit on. Every other registeredGroup value now falls through unconditionally to the disk-scan-based check, which is always accurate for non-compile-time plugins. Verified by temporarily reverting the fix locally and confirming the new STALE test fails for exactly the right reason (throws the "uninstall the old one first" error when it should resolve cleanly), then restoring it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2
|
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the prior false-block finding is fixed.
Reviewed changes
- Non-core
registeredGroupshort-circuit removed —assertNoConflictingInstallnow early-throws only whenregisteredGroup === 'core'; every other registry group falls through to the on-disklistInstalledPlugins()scan, so a stale registry record left by an in-session uninstall no longer false-blocks a legitimate reinstall, while a real non-core conflict still throws. - Tests updated — the old non-core short-circuit test is replaced by a stale-record case (empty disk → resolves) plus a real-conflict case (disk still holds the other group → throws), and both fail for the right reason if the branch regresses.
The remaining open thread from the 2414485 review (the PR body still describes the storage namespace as "unchanged from before this PR" for market installs) is untouched by this commit and keeps this review non-approving.
Important
Pullfrog covered this run's model usage. DeepSeek Flash is fast and cheap — expect lighter work than a frontier model. This model allowance is temporary and separate from your Pullfrog plan. Connect a model-provider subscription or API key →
DeepSeek Flash (model usage covered by Pullfrog) ｜ 𝕏
DianaSensei
left a comment
DianaSensei
left a comment
There was a problem hiding this comment.
Good catch — the PR body's "unchanged from before this PR" claim was inaccurate; updated it to correctly describe the namespace as changed for market installs, with the actual justification for why no data migration is needed (market installs crashed in usePluginSdkFor before ever reaching a storage/secrets call under the old scheme, so there's no data to lose) — same reasoning already in the inline comment above installedPluginManifests() in installer.ts.
Generated by Claude Code
2 participants
Summary
A plugin installed via a market (e.g. Containers from the official market) opened to
This tool hit an error—Không có plugin "container-manager" trong registry. usePluginSdkFor() chỉ dùng cho code thuộc một plugin có thật, không phải cho shell.— even though it was installed and actively rendering.Root cause:
installer.tsprefixed a market-installed plugin's registry id with<marketId>-(e.g.official-container-manager) so two markets couldn't collide over the same plugin id. But the plugin's own bundled code has no idea about that prefix — it always callsusePluginSdkFor('container-manager')/getPluginSdk('container-manager')with its bare, self-authored id.Design
Every plugin now has two identity fields:
id— self-authored, bare, what the plugin's own bundle calls itself viausePluginSdkFor/getPluginSdk. Also the storage/secrets/audit namespace key. This is a change from the old<marketId>-<id>scheme for market installs — but it needs no data migration: under the old scheme, a market-installed plugin crashed inusePluginSdkFor(the bug this PR fixes) before it could ever reach a singlesdk.storage/sdk.secretscall, so there is no data under the old namespace for anyone to lose. See the comment aboveinstalledPluginManifests()ininstaller.tsfor the same reasoning inline.group— where it was installed from:"core"for the 26 compile-time plugins (auto-assigned, no need to touch those files),"url"for a bare-URL install, or themarketIdfor a market install. Pure metadata: used for route generation (/installed/<group>/<id>, unified for both URL and market installs) and for deciding install-time conflicts.idstill ends up unique system-wide in practice:usePluginSdkFor/getPluginSdkare called by a plugin's own bundle with just the bare id, no group — a module-scope call (Kafka's/RabbitMQ's background consumer stores, no React context) can't resolve which group it belongs to. Letting the same id genuinely coexist under two groups would make that lookup ambiguous and silently misattribute storage/audit/permissions to the wrong plugin.So
installer.tsenforces it at install time, not registry time:assertNoConflictingInstall(id, marketId, registeredGroup?)checks already-installed plugins for the same bareidunder a different group and throws a clear "uninstall the old one first" error before Rust ever writes to disk — called from both install call sites (ExtensionInstallDialog,SettingsExtensionInstaller) using the manifest they've already fetched for the confirmation screen (no extra network round-trip). The optionalregisteredGroupcatches the same-id-as-a-built-in-tool case immediately; every other group value falls through to an on-disk scan, since the in-memory registry only loads once at bootstrap and can go stale mid-session (e.g. right after an in-session uninstall).registry.tsis back to the simple pre-bug form:registerManifestjust defaultsgroupto'core'for compile-time plugins and keeps one flatseenIdsuniqueness check;getPlugin(id)is a plainPLUGIN_MAP.get(id)— no fallback/ambiguity machinery left.Test plan
tsc --noEmitvitest run— 1376/1376 passingassertNoConflictingInstall's guards by temporarily reverting each fix — the corresponding tests fail for the right reason, confirming real coverage🤖 Generated with Claude Code
https://claude.ai/code/session_01BCypCuViyQDKGWxWspXKs2