Feat/linkcode plugin marketplace

[adminliu-main]

Summary

Verification

Checklist

• pnpm check:ci and pnpm test both pass (plus cargo fmt / clippy / test for Rust changes)
• I ran the affected surface and observed the change working
• If a wire message changed: WIRE_PROTOCOL_VERSION is bumped
• New code and assets are my own work, or their origin and license compatibility are noted above
• Docs and comments are updated where behavior changed

[greptile-apps]

Greptile Summary

The PR adds an end-to-end LinkCode plugin marketplace, including catalog refresh, package installation, plugin configuration, client APIs, and UI integration. The concurrent-install fix isolates temporary extraction state but still leaves final package publication shared.

• Adds marketplace schemas, wire operations, daemon and host services, caching, and installation.
• Adds plugin configuration with vault-backed secrets and workbench settings UI.
• Adds the mail MCP plugin, marketplace development tooling, and broad unit/E2E coverage.

Confidence Score: 4/5

The PR is not yet safe to merge because concurrent installs of the same release can still delete a successfully published plugin package.

Unique extraction directories fix the original shared staging collision, but final publication remains an uncoordinated remove-and-rename operation on one shared target directory, so an overlapping install can remove another request's committed package and leave it absent if publication then fails.

Files Needing Attention: apps/daemon/src/plugin-store/store.ts, apps/daemon/src/plugin-store/paths.ts

Important Files Changed

Filename	Overview
apps/daemon/src/plugin-store/paths.ts	Adds UUID-based staging paths, correctly separating extraction state but not final publication.
apps/daemon/src/plugin-store/store.ts	Implements plugin installation and configuration rollback, but same-release installs still race while replacing the shared final package directory.
apps/daemon/src/config.ts	Adds marketplace and non-secret plugin configuration parsing and durable persistence.
apps/daemon/src/marketplace/service.ts	Adds validated marketplace refresh, cache, ETag, and release-resolution behavior.
packages/foundation/schema/src/model/linkcode-plugin.ts	Defines constrained plugin identities, versions, manifests, releases, and settings contracts.
packages/host/engine/src/plugin/market-request-handler.ts	Exposes marketplace operations through the engine request boundary.

Sequence Diagram

sequenceDiagram
  participant A as Install A
  participant B as Install B
  participant T as Shared targetDir
  A->>A: Extract into unique staging A
  B->>B: Extract into unique staging B
  A->>T: Remove targetDir
  A->>T: Rename staging A to targetDir
  B->>T: Remove targetDir
  Note over B,T: Deletes A's committed package
  B->>T: Rename staging B to targetDir
  alt Rename fails
    B-->>B: Request fails
    Note over T: Installed package remains absent
  end

Loading

_{Reviews (2): Last reviewed commit: "fix(daemon): harden plugin install stagi..." | Re-trigger Greptile}

[pullfrog]

Important

The marketplace core is solid, but the one plugin a user can actually reach — the mail MCP template — points at an unpublishable package, so the happy path dead-ends. A few state-management bugs (catalog keys, upgrade path, registry keying, secret pruning) are worth fixing before this ships.

Reviewed changes

• Read the complete diff end to end: 76 files, ~6,800 lines.
• Verified the wire-version call: WIRE_PROTOCOL_VERSION 78 → 79 with MIN_COMPATIBLE_WIRE_VERSION untouched is correct — every new frame is purely additive.
• Traced the secret path end to end and cleared it: maskValues in plugin/config-service.ts does if (field.secret) continue, both plugin-config.listed and plugin-config.updated route through it, and the e2e test asserts no leak.
• Cleared package-path traversal: LinkCodePluginPackagePathSchema rejects leading /, \, segments ending in ., and Windows reserved names — this covers component.entry too. SRI verification via downloadVerified fails closed.
• Confirmed packages/integrations/mail-mcp is in the root tsconfig.json references, so it is not silently unchecked.

⚠️ The only reachable plugin template installs a package that does not exist

This is the finding I'd most want resolved, and it spans more than one line so it doesn't fit inline.

@linkcode/mail-mcp is "private": true at "version": "0.0.0", and a scan of every package.json in the workspace found zero publishable packages. The mail template in mcp-settings.tsx prefills npx -y @linkcode/mail-mcp, and scripts/dev-marketplace.mts packs the same dist into a local tgz.

So there are two delivery stories in this PR and neither reaches a real user:

• The template shells out to npx, which will hit the public registry and fail.
• The dev marketplace works, but only over loopback for a developer running the script.

What's missing is the story for how https://plugins.linkcode.ai/index.json — the production default in config.ts — ever comes to host this artifact. Is a publish step planned, is the template meant to be dev-only for now, or should the mail plugin ship through the marketplace rather than the custom-MCP form? That's a scope question only you can answer.

⚠️ Scope: two features in one PR

packages/integrations/mail-mcp is ~1,400 lines of IMAP/SMTP client that is independent of the marketplace machinery — it's a consumer of it, not part of it. AGENTS.md targets ~200 changed lines per commit with a hard limit of 400; this PR is ~6,800.

I'm not asking you to unpick work that's already done, but landing the marketplace and the mail server as separate PRs would make both reviewable, and would let the marketplace ship without waiting on the publish question above. If they must land together, it's worth saying why in the PR description.

ℹ️ PR description is empty

Given the size and the two independent features, a description covering the rollout order (daemon before clients?), the publish plan for plugin artifacts, and what was manually verified would help reviewers and future archaeology.

ℹ️ Verification I could not complete

Worth flagging so it isn't mistaken for a pass. This checkout's node_modules predates the branch, so imapflow and nodemailer are absent from disk. pnpm test therefore reports 3 failed files against 354 passed (3078 tests passing), and all three failures are ERR_MODULE_NOT_FOUND for those two packages — an environment artifact, not a defect in your change. Root pnpm typecheck is a cold full-solution tsc --build that did not finish in the review window and would emit the same false missing-module errors.

Please confirm pnpm check:ci and pnpm test both pass after a fresh pnpm install, since I could not establish either gate.

ℹ️ Nitpicks

• mail-mcp/src/index.ts: const VERSION = '0.0.0' duplicates the version in package.json and will drift.
• mail-mcp/src/index.ts: in the shutdown handler, if (signal) process.exit(0) — signal is always truthy there, so the conditional is dead.
• mail-mcp/package.json declares a "typecheck": "tsc --noEmit" script. Per AGENTS.md there are no per-package typecheck scripts; the root solution build covers this package already.
• mail-mcp/src/config.ts: there's no IMAP_PORT/SMTP_PORT override — a user on a non-standard port has no way to reach it, since ports come only from a preset or a protocol default.
• mail-mcp/src/smtp.ts: the secure: false path doesn't set requireTLS, so a server that fails to offer STARTTLS will silently proceed in plaintext rather than refusing.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

@linkcode/mail-mcp is "private": true at version 0.0.0, and no package in this workspace is publishable. npx -y @linkcode/mail-mcp will therefore try the public registry and fail, so anyone who picks a mail preset gets a saved MCP server that can never start — and the failure surfaces later, at session start, not here in the dialog where they could act on it.

See the review body for the broader distribution question.

[pullfrog]

The card key omits the version, but flattenReleases in apps/daemon/src/marketplace/service.ts emits every release of every plugin:

return index.plugins.flatMap((plugin) =>
  plugin.releases.map((release) => ({ pluginId: plugin.id, release })),
);

So a plugin with two published releases yields two cards with an identical key. That key is consumed directly as the React key at packages/presentation/ui/src/shell/plugins/linkcode-catalog.tsx:186 (<CatalogCard key={card.key} …>), which means duplicate-key warnings and unstable reconciliation — the install button's busy state can attach to the wrong card.

Including the version makes each row unique:

Suggested change

	key: `${marketplaceId}:${entry.pluginId}`,
	key: `${marketplaceId}:${entry.pluginId}:${manifest.version}`,

[pullfrog]

installedIds is keyed on plugin id alone, so once any version of a plugin is installed, every release of it renders as already-installed. There's no way to upgrade from the catalog — the user has to uninstall and reinstall, which (given uninstall calls savePluginConfigValues(pluginId, {}) and prunePluginSecrets) destroys their configuration and secrets in the process.

Comparing the installed version against entry.release.manifest.version would let you distinguish installed / upgradable / not-installed. Worth confirming whether an upgrade path is in scope for this PR or deliberately deferred.

[pullfrog]

secretFieldIds re-derives the surviving key set by reading each plugin's manifest back off disk, and readManifest returns undefined on a missing file, unparseable JSON, or a manifest that fails schema validation — in which case this returns [].

Because the result feeds secrets.replaceAll(surviving), that empty list isn't a no-op: it means an unrelated plugin's secrets are silently deleted whenever any other plugin is uninstalled. A plugin whose manifest was corrupted, or whose schema drifted after a daemon upgrade, quietly loses its stored credentials, and the only trace is the Dropping invalid plugin manifest warn logged from readManifest.

Safer to prune by key prefix rather than reconstructing the whole set from manifests — i.e. keep every existing plugin key that doesn't start with ${pluginId}. — so an unreadable manifest can't cause collateral loss.

[pullfrog]

This filter is a tautology. artifact.urls is typed from z.array(z.union([z.url(...), LinkCodePluginPackagePathSchema])) and both union branches produce string, so typeof url === 'string' is always true and httpsUrls is always just artifact.urls.

That makes the guard on the next line unreachable, and its message ('Plugin release has no HTTPS download URL') misleading — the schema's .min(1) already guarantees at least one entry, and a relative package path would pass this check while not being an HTTPS URL at all.

If the intent is to reject non-HTTPS entries after resolveMirrorUrl has absolutized them, the predicate needs to actually test the scheme.

[pullfrog]

if (settingId in settings) means an unconfigured setting is silently omitted from the child process env. For a required credential — MAIL_USER / MAIL_PASSWORD for the mail server — the result is that the MCP server spawns, loadConfig throws ConfigError, and the process exits 1 with nothing surfaced to the user.

Everything else in this resolver reports problems through warnings (the name-conflict case just above does exactly that). Pushing an McpWarning when a manifest-declared setting has no value would let the user see why the server didn't come up, rather than finding an empty tool list.

[pullfrog]

"Must be an absolute HTTPS URL" is no longer accurate — isAllowedMarketplaceUrl in packages/foundation/schema/src/model/linkcode-marketplace.ts deliberately also accepts plain http on loopback, which is what makes scripts/dev-marketplace.mts usable.

The zod refine message in that same file has drifted for the same reason:

.refine((url) => isAllowedMarketplaceUrl(url), 'Expected an absolute HTTPS URL');

A developer pointing LINKCODE_MARKETPLACE_URL at their local server gets told HTTPS is required by both the docs and the error text, even though it works. Worth updating both to mention the loopback exemption.

[pullfrog]

This says notModified replies "carry no releases", but the daemon does the opposite — apps/daemon/src/marketplace/service.ts:92 returns releases: cachedIndex === undefined ? [] : flattenReleases(cachedIndex) alongside notModified: true, so the cached catalog is carried.

packages/client/core/src/client.ts:956 documents it correctly ("carry the cached catalog"), and operations.ts:358 repeats this file's incorrect version — so three files disagree in two directions.

The defensive merge in linkcode-tab.tsx is commented "for older daemons that still return an empty 304 payload", which suggests this doc is simply stale rather than describing current behavior. Worth correcting both SDK comments so callers don't write unnecessary fallbacks.

[greptile-apps]

Shared package publication race

If two clients install the same plugin version concurrently, both requests publish to the same targetDir using rmSync followed by renameSync. The later request deletes the package committed by the first, and a subsequent rename failure leaves the package absent while the registry can still identify it as installed.

[pullfrog]

Important

Incremental review of 6373d0c. The registry-keying issue from the last review is genuinely fixed; the new disabled marketplace gate is only half-wired.

6373d0c resolves exactly one of the nine findings from the previous review: plugin-store/store.ts registry keying. upsertRegistry now keys on id alone, uninstall removes every matching row plus its package dir, currentRegistryRecords makes legacy duplicates resolve newest-wins, and install GCs stale package dirs after the new record lands. That thread is resolved.

The other eight prior threads remain open and are not repeated here — most notably the prunePluginSecrets cross-plugin secret destruction (store.ts ~313-325), the npx -y @linkcode/mail-mcp template pointing at a private: true package, and the duplicate-React-key / no-upgrade-path pair in view.ts + linkcode-tab.tsx. They still need answers before merge.

New in this commit, three inline notes. The first is the one worth acting on: the daemon now rejects plugin-market.refresh/install for a disabled marketplace with code: 'forbidden', but the client still renders disabled marketplaces exactly like enabled ones and never reads the error — so turning a marketplace off leaves a section stuck on loading skeletons forever.

Also re-verified as not defects this pass, so no need to chase them:

• The retriedWithoutValidators flag bounds the new 304-without-cache recursion at one retry; a parseIndex failure still leaves the previous cache intact.
• code: 'forbidden' is wire-valid — request.failed types code as z.string().optional(), so there is no enum to extend.
• LinkCodePluginManifestReaderSchema is a clean single export; the range-diff rendering that appeared to duplicate the const line is an artifact.
• imapflow does emit 'close' (lib/imap-flow.js:2829, typed at imap-flow.d.ts:933), so attachLifecycleHandlers' close handler is live code, not a no-op.
• The Chinese description in scripts/dev-marketplace.mts is a plugin-manifest field in a dev fixture, not an untranslated i18n key.

Scope note from the last review still stands: packages/integrations/mail-mcp is a second, independently shippable feature inside an already ~7k-line PR, and the PR body is still empty. Splitting it would make both halves reviewable against AGENTS.md's ~200-line target.

_{Files reviewed: 77 · Commits reviewed: 3 · Base: master · Head: feat/linkcode-plugin-marketplace (6373d0c) · Prior review: 2509c6c}

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Claude Opus ｜ 𝕏}

[pullfrog]

This commit added a daemon-side gate — market-request-handler.ts now fails plugin-market.refresh and plugin-market.install with code: 'forbidden' when enabled === false — but the client half is missing, so a disabled marketplace degrades into a section stuck on loading skeletons.

marketplaces comes straight from usePluginMarketplaces() with no enabled filter, so every configured marketplace still gets a MarketplaceCatalog. Inside it, usePluginMarketCatalog(marketplace.id) destructures only { data, isLoading, isValidating, mutate } — no error — and onRefresh ends in .catch(noop). So data stays undefined → cards stays undefined, and LinkCodeCatalogSection renders the cards === undefined branch, which is SKELETON_ROWS (linkcode-catalog.tsx:173-178). The user gets three shimmering placeholders that never resolve, plus a refresh button that silently does nothing. There is no catalogDisabled-style string in the new locale block either.

enabled is on the wire (linkcode-marketplace.ts:45), so the filter is cheap:

Suggested change

	marketplaces.map((marketplace) => (
	marketplaces
	.filter((marketplace) => marketplace.enabled)
	.map((marketplace) => (

If disabled marketplaces are meant to stay visible so the user can see what they turned off, the section needs an explicit disabled state and a locale string instead. Either is fine — but perpetual skeletons over a request the daemon is now guaranteed to reject is the one outcome that reads as a hang.

[pullfrog]

The resolveRelease half of this test is vacuous — it passes whether or not the guard exists.

beforeEach points HOME at a fresh mkdtempSync dir per test, and the refresh on line 163 throws on the enabled check before any fetch, so nothing ever writes the index cache. resolveRelease then short-circuits on index === undefined and never evaluates !config?.enabled. Deleting !config?.enabled from marketplace/service.ts:137 leaves this test green.

To actually cover it, populate the cache through an enabled config first, then build a second service over the same HOME with enabled: false:

const fetchIndex = vi.fn(() => Promise.resolve(fakeResponse(200, JSON.stringify(INDEX))));
await new DaemonLinkCodeMarketplaceService(MARKETPLACES, fetchIndex).refresh('linkcode-official');

const disabled: LinkCodeMarketplaceConfigList = [{ ...MARKETPLACES[0], enabled: false }];
const service = new DaemonLinkCodeMarketplaceService(disabled, fetchIndex);
expect(
  service.resolveRelease({ marketplaceId: 'linkcode-official', pluginId: 'arcbox/latex', version: '1.2.0' }),
).toBeUndefined();

Worth getting right because resolveRelease is what install consults — it is the layer that stops an already-cached release from being installed out of a marketplace the user has since turned off, which is exactly the case the request-handler's forbidden check cannot see.

[pullfrog]

Assigning transport.onclose after server.connect(transport) overwrites the SDK's own handler instead of adding to it.

Protocol.connect (@modelcontextprotocol/sdk/dist/esm/shared/protocol.js:219-223) installs a composite:

const _onclose = this.transport?.onclose;
this._transport.onclose = () => { _onclose?.(); this._onclose(); };

It captures whatever was already there and chains it, so the supported pattern is to set onclose before connecting. Setting it on line 51 replaces that closure wholesale, so Protocol._onclose() — which clears _responseHandlers/_progressHandlers and rejects every in-flight request — never runs. The re-entrancy guard then hides it on the shutdown path too: shutdown() calls server.close() → transport.close() → our onclose → shutdown() again, which returns immediately because shuttingDown is already true.

Move the assignment above the connect and let the SDK chain it:

const transport = new StdioServerTransport();
transport.onclose = () => shutdown(true);
await server.connect(transport);

That needs the shutdown declaration hoisted above the connect as well — a straight reordering of the existing block, no logic change.