fix(plugins): Make plugin loading work end to end - #1386
Open
TimothyJones wants to merge 4 commits into
Open
Conversation
…e packaged Previously, loading a plugin by name failed for plugins built with `export default plugin` or `module.exports = plugin`, because the dynamic import()'s module namespace never exposes the plugin's properties directly. Now the module contents are unwrapped with the new mustResolvePlugin helper (in case-plugin-base), which accepts every packaging style and throws a helpful INVALID_PLUGIN_MODULE configuration error when a module doesn't contain a plugin. Additionally: - Plugin names are now resolved against the working directory (the user's project) first, so plugins are found when the connector runs from a temporary directory (eg when called from the Java DSL) - Falls back to require() in environments that can't do a dynamic import() (eg Jest without --experimental-vm-modules) - PluginLoader now validates the plugin objects it's given (previously a TODO) - Unsafe module specifiers now surface as a BoundaryResult failure rather than a synchronous throw across the never-throws boundary Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014mD6p9EEmftbpBQHZZBWB1
…fier Mirrors the Java DSL's loadPlugins methods, so that JS/TS users can load plugins too. Includes end-to-end tests that load a fixture plugin by package name and use its matcher during contract definition and verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014mD6p9EEmftbpBQHZZBWB1
…s it Adds TypeScript examples to the loading documentation, removes the caveat about the JS DSL not exposing loadPlugins, and ticks off the plugin loading items in the maintainer todo list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014mD6p9EEmftbpBQHZZBWB1
… definer The definer's loadPlugins built its request by calling .addAll() on the protobuf builder's unmodifiable module names list view, so the module names were never sent to the connector. Now uses .addAllModuleNames(), matching the verifier. Also adds an end-to-end test that loads a fixture plugin by package name, proving that plugins are resolved from the project's node_modules even though the connector runs from a temporary directory - and corrects the loadPlugins javadoc, which claimed paths were supported (they aren't, for security reasons). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014mD6p9EEmftbpBQHZZBWB1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1385 (
claude/plugin-docs-update-ck8mgs) — fixes three of the plugin todos raised while writing the plugin documentation, plus a bug found along the way.Module namespace unwrapping
A dynamic
import()namespace never exposesdescriptionetc for plugins built withexport defaultormodule.exports = plugin(only individual named exports worked), so the documented "default export oneContractCasePluginobject" shape could never actually load. NewmustResolvePlugin/isContractCasePluginhelpers incase-plugin-baseaccept every packaging style (ESM default, CJSmodule.exports, TypeScript-compiled CJS default, named exports), with a new documentedINVALID_PLUGIN_MODULEerror code for modules that aren't plugins.PluginLoader'sTODO: Validate plugins hereis implemented with the same guard.Project-local resolution (the Java case)
Plugin names are now resolved against the working directory first, falling back to normal resolution. This is what makes plugins findable when the connector runs from a temporary directory, as it does for Java. There's also a
require()fallback for environments where dynamicimport()isn't available — notably Jest without--experimental-vm-modules, which TS DSL users would otherwise hit.Additionally, the specifier validation moved inside the promise chain: previously an invalid name threw synchronously out of the never-throws boundary instead of producing a
BoundaryFailure.loadPluginsin the TS DSLContractCaseDefiner.loadPlugins(...)andContractVerifier.loadPlugins(...), mirroring the Java DSL semantics (DEFINER_LOAD_PLUGIN/VERIFICATION_LOAD_PLUGIN).BoundaryPluginLoaderis now exported fromcase-connectorto support this. No adapter mirroring needed — the jest/vitest adapters re-export the shared classes.Java definer bug
InternalDefinerClient.loadPluginsbuilt its request by calling.addAll()on the protobuf builder's unmodifiablegetModuleNamesList()view, so the definer never actually sent the module names. Fixed to.addAllModuleNames(...), matching the verifier. The stale javadoc ("Can be a path to the package") is also corrected — paths are intentionally rejected.Tests
case-connectorvitest tests over a committed fixture plugin package (test-fixtures/contract-case-test-plugin) with one entry point per packaging style, covering load success, idempotency, non-plugins, missing packages, and unsafe specifiersPluginLoadTestindsl-java: the full chain — Java → gRPC → temp-dir connector → projectnode_modules→ unwrap → matcher registered and matched — plus a negative test. The full Java suite passes.The user-facing docs from the base PR are updated in place (TS
loadPluginsexamples added, "not yet supported" caveat removed), and the corresponding maintainer todos are ticked.🤖 Generated with Claude Code
https://claude.ai/code/session_014mD6p9EEmftbpBQHZZBWB1