Skip to content

refactor: migrate claude to the target registry (no behavior change) - #15

Closed
steve-calvert-glean wants to merge 1 commit into
refactor/cursor-target-registryfrom
refactor/claude-target-registry
Closed

refactor: migrate claude to the target registry (no behavior change)#15
steve-calvert-glean wants to merge 1 commit into
refactor/cursor-target-registryfrom
refactor/claude-target-registry

Conversation

@steve-calvert-glean

Copy link
Copy Markdown
Contributor

Summary

  • Ports emitClaude/validateClaude into src/targets/claude.ts as a PluginTargetDefinition, following the pattern established in fix: correct Copilot and Antigravity plugin output + polymorphic target registry #13/refactor: migrate cursor to the target registry #14.
  • Verified directly against claude plugin validate --strict: a minimal manifest with only name fails that check, confirming the existing version/description/author.name requirements already match the real CLI. No validation behavior change — this is a structural port.
  • Applies the per-plugin version override in buildPluginManifest (pluginConfig.version ?? version) from the start, avoiding the bug just fixed on cursor's copy of this pattern in refactor: migrate cursor to the target registry #14 (a new regression test covers this: "applies per-target and per-plugin version overrides").
  • Adds @deprecated markers to the now-superseded emitClaude/validateClaude, matching copilot/antigravity's already-migrated pattern.

Stacking

Based on refactor/cursor-target-registry (#14), since claude's resolvePluginPath/manifestPaths/marketplacePaths need the same targetConfig parameter that cursor's migration introduced on the shared interface. Merge #14 first.

Test plan

  • npm run typecheck
  • npm run lint
  • npm test (59/59 passing, including claude's pre-existing version-override regression test)
  • npm run build
  • node dist/cli.js docs --check

Ports emitClaude/validateClaude into src/targets/claude.ts as a
PluginTargetDefinition, verified directly against `claude plugin
validate --strict`: a minimal manifest with only `name` fails that
check, confirming the existing version/description/author.name
requirements already match the real CLI rather than over-constraining
it.

Applies the per-plugin version override in buildPluginManifest
(pluginConfig.version ?? version) up front, matching the identical fix
just made to cursor's copy of this pattern.
Comment thread src/targets/claude.ts
pluginConfig.path ??
path.join(targetConfig.pluginRoot ?? "plugins", pluginName),

buildPluginManifest: ({ metadata, version, pluginName, pluginConfig }) =>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: The new Claude target implementation in src/targets/claude.ts does not apply manifest override objects (pluginConfig.manifest and targetConfig.manifest) to the generated plugin and marketplace manifests, unlike the legacy claudePluginManifest and emitClaude logic in src/targets.ts, so user-specified manifest overrides will no longer take effect once the registry-based Claude target is used.

Suggested fix: Update the claude target in src/targets/claude.ts so that buildPluginManifest deep-merges the base manifest with pluginConfig.manifest and buildMarketplaceManifest deep-merges the base marketplace object with targetConfig.manifest, mirroring the existing behavior of claudePluginManifest and emitClaude in src/targets.ts (which use deepMerge(..., pluginConfig.manifest ?? {}) and deepMerge(..., targetConfig.manifest ?? {})) before calling stripUndefined.

🔧 Tag @ glean-for-engineering to fix or click here to fix in Glean

💬 Help us improve! Was this comment helpful? React with 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Internal refactoring, cleanup, or maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant