Skip to content

fix(mcp): add missing pipeline step types and dynamic plugin type discovery#196

Merged
intel352 merged 5 commits intomainfrom
copilot/test-mcp-server-discoveries
Feb 27, 2026
Merged

fix(mcp): add missing pipeline step types and dynamic plugin type discovery#196
intel352 merged 5 commits intomainfrom
copilot/test-mcp-server-discoveries

Conversation

Copy link
Contributor

Copilot AI commented Feb 27, 2026

  • Investigate issue: step.foreach missing from coreModuleTypes; plugin types not visible in MCP server
  • Fix 1: Add 15 missing step types from pipelinesteps plugin to coreModuleTypes in schema/schema.go
  • Fix 2: Add RegisterTriggerType/RegisterWorkflowType dynamic registries to schema/schema.go and make KnownTriggerTypes()/KnownWorkflowTypes() dynamic
  • Fix 3: In MCP server, load installed plugin manifests from pluginDir at startup to register plugin-provided types (module, step, trigger, workflow)
  • Fix 4: Enhance list_plugins to show types per plugin from their manifests
  • Add schema definitions for the 15 new step types in module_schema.go
  • Tests: Added tests for step.foreach presence, plugin dir module/step/trigger/workflow type discovery, and per-plugin type listing
  • Fix review feedback (round 1): correct config field names in 7 new step schemas to match actual implementations
  • Fix review feedback (round 2): correct remaining step schema fields
    • KnownTriggerTypes/KnownWorkflowTypes: always return sorted slice (consistent ordering even without dynamic types)
    • step.webhook_verify: document both scheme-based and legacy provider-based config models
    • step.cache_get: output key (not output_key), cache required, add miss_ok
    • step.cache_set: value key (not value_from), cache required
    • step.cache_delete: cache now required
    • step.event_publish: topic required (not event_type), remove payload_from, add headers/broker
    • step.validate_pagination: default_page/default_limit/max_limit (not default_page_size/max_page_size)
    • step.dlq_send: topic required (not queue), remove reason_from/payload_from, add original_topic/error/payload/broker
    • step.dlq_replay: dlq_topic+target_topic required (not queue/limit), add max_messages/broker
  • All tests pass; golangci-lint reports 0 issues

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…P server

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot AI changed the title [WIP] Add findings from MCP server testing fix(mcp): add missing pipeline step types and dynamic plugin type discovery Feb 27, 2026
@intel352 intel352 marked this pull request as ready for review February 27, 2026 05:59
Copilot AI review requested due to automatic review settings February 27, 2026 05:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a critical gap in the MCP server's type listings by adding 15 missing built-in pipeline step types to coreModuleTypes and implementing dynamic plugin type discovery. The MCP server can now load plugin manifests at startup and register their declared module, step, trigger, and workflow types, making plugin-contributed types (e.g., auth.m2m, step.authz_check) visible in type listings.

Changes:

  • Added 15 missing step types to coreModuleTypes in alphabetical order: step.foreach, step.webhook_verify, cache operations, event/DLQ operations, resilience patterns, validation steps, and UI scaffold steps
  • Implemented dynamic type registration for triggers and workflows with RegisterTriggerType/RegisterWorkflowType functions mirroring the existing module type pattern
  • Added loadInstalledPluginTypes() in MCP server to scan plugin manifests and register all type declarations at startup
  • Enhanced list_plugins response to include per-plugin type declarations
  • Added comprehensive test coverage for both built-in types and plugin directory discovery

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

File Description
schema/schema.go Added 15 step types to coreModuleTypes; implemented dynamic trigger/workflow type registration; updated KnownTriggerTypes/KnownWorkflowTypes to merge dynamic types
schema/module_schema.go Added ModuleSchema definitions for all 15 new step types with config fields, descriptions, and metadata
mcp/server.go Implemented loadInstalledPluginTypes to read plugin manifests; updated tool descriptions; enhanced list_plugins to include type declarations
mcp/server_test.go Added tests verifying new step types presence; added plugin directory tests for all four type categories; added test for list_plugins type enrichment

@intel352
Copy link
Contributor

@copilot apply changes based on the comments in this thread

- step.foreach: items_from→collection; add item_var/item_key/index_key; step/steps as maps
- step.validate_path_param: param→params (array); add format and source fields
- step.validate_request_body: replace schema/output_key with required_fields (actual impl)
- step.retry_with_backoff: step_config removed, max_attempts→max_retries, add multiplier; step is map
- step.resilient_circuit_breaker: step_config removed, threshold→failure_threshold,
  timeout→reset_timeout, add fallback map field
- step.ui_scaffold: replace prompt_from/output_key with title/theme/auth/filename (actual impl)
- step.ui_scaffold_analyze: replace target_from/output_key with title/theme (actual impl)

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

Comment on lines +278 to +295
// KnownWorkflowTypes returns all built-in workflow handler type identifiers plus any types
// registered at runtime by plugins. The result is sorted and deduplicated.
func KnownWorkflowTypes() []string {
return []string{
core := []string{
"event",
"http",
"messaging",
"statemachine",
"scheduler",
"integration",
}

dynamicWorkflowMu.RLock()
defer dynamicWorkflowMu.RUnlock()

if len(dynamicWorkflowTypes) == 0 {
return core
}
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

KnownWorkflowTypes() docstring says the result is sorted and deduplicated, but when no dynamic workflow types are registered it returns the unsorted core slice as-is. This makes ordering inconsistent depending on whether plugins are present. Consider always returning a sorted copy (or calling sort.Strings(core) before returning) so callers get deterministic ordering.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. KnownWorkflowTypes now always returns a sorted copy regardless of whether dynamic types are registered.

Comment on lines +1620 to +1624
{Key: "provider", Label: "Provider", Type: FieldTypeSelect, Options: []string{"github", "stripe", "generic"}, Description: "Webhook provider (legacy; prefer scheme)"},
{Key: "scheme", Label: "Scheme", Type: FieldTypeSelect, Options: []string{"hmac-sha1", "hmac-sha256", "hmac-sha256-hex"}, Description: "HMAC signature scheme to use"},
{Key: "secret", Label: "Secret", Type: FieldTypeString, Sensitive: true, Description: "Webhook signing secret"},
{Key: "secret_from", Label: "Secret From", Type: FieldTypeString, Description: "Context key containing the secret at runtime"},
{Key: "signature_header", Label: "Signature Header", Type: FieldTypeString, Description: "HTTP header containing the signature", Placeholder: "X-Hub-Signature-256"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.webhook_verify schema fields do not match the current implementation. module/WebhookVerifyStep factory requires provider and secret, and optionally accepts header; it does not read scheme, secret_from, or signature_header. The schema should reflect the actual supported config keys to avoid generating invalid configs.

Suggested change
{Key: "provider", Label: "Provider", Type: FieldTypeSelect, Options: []string{"github", "stripe", "generic"}, Description: "Webhook provider (legacy; prefer scheme)"},
{Key: "scheme", Label: "Scheme", Type: FieldTypeSelect, Options: []string{"hmac-sha1", "hmac-sha256", "hmac-sha256-hex"}, Description: "HMAC signature scheme to use"},
{Key: "secret", Label: "Secret", Type: FieldTypeString, Sensitive: true, Description: "Webhook signing secret"},
{Key: "secret_from", Label: "Secret From", Type: FieldTypeString, Description: "Context key containing the secret at runtime"},
{Key: "signature_header", Label: "Signature Header", Type: FieldTypeString, Description: "HTTP header containing the signature", Placeholder: "X-Hub-Signature-256"},
{Key: "provider", Label: "Provider", Type: FieldTypeSelect, Options: []string{"github", "stripe", "generic"}, Description: "Webhook provider"},
{Key: "secret", Label: "Secret", Type: FieldTypeString, Sensitive: true, Description: "Webhook signing secret"},
{Key: "header", Label: "Signature Header", Type: FieldTypeString, Description: "HTTP header containing the signature", Placeholder: "X-Hub-Signature-256"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. Schema now documents both config models: scheme-based (scheme, secret, secret_from, signature_header) and legacy provider-based (provider, secret, header).

Comment on lines +1635 to +1636
{Key: "output_key", Label: "Output Key", Type: FieldTypeString, Description: "Context key for the retrieved value", DefaultValue: "cache_value"},
{Key: "cache", Label: "Cache Module", Type: FieldTypeString, Description: "Name of the cache module to use"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.cache_get schema uses output_key and makes cache optional, but the implementation requires cache and uses the key output (default "value"); it also supports miss_ok. Please align the schema field keys/requiredness with module/CacheGetStep so generated configs validate and work at runtime.

Suggested change
{Key: "output_key", Label: "Output Key", Type: FieldTypeString, Description: "Context key for the retrieved value", DefaultValue: "cache_value"},
{Key: "cache", Label: "Cache Module", Type: FieldTypeString, Description: "Name of the cache module to use"},
{Key: "output", Label: "Output Key", Type: FieldTypeString, Description: "Context key to store the retrieved value", DefaultValue: "value"},
{Key: "cache", Label: "Cache Module", Type: FieldTypeString, Required: true, Description: "Name of the cache module to use"},
{Key: "miss_ok", Label: "Allow Cache Miss", Type: FieldTypeBool, Description: "If true, do not fail when the cache key is missing"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. output_keyoutput (default "value"), cache is now required, and miss_ok is documented.

Comment on lines +1692 to +1695
Description: "Validates and normalizes pagination query parameters (page, page_size, limit, offset)",
ConfigFields: []ConfigFieldDef{
{Key: "default_page_size", Label: "Default Page Size", Type: FieldTypeNumber, DefaultValue: 20, Description: "Default number of items per page"},
{Key: "max_page_size", Label: "Max Page Size", Type: FieldTypeNumber, DefaultValue: 100, Description: "Maximum allowed page size"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.validate_pagination schema fields (default_page_size, max_page_size) do not match the implementation, which reads default_page, default_limit, and max_limit. As written, the schema will generate/validate configs that the step ignores. Please update the schema keys to match module/ValidatePaginationStep.

Suggested change
Description: "Validates and normalizes pagination query parameters (page, page_size, limit, offset)",
ConfigFields: []ConfigFieldDef{
{Key: "default_page_size", Label: "Default Page Size", Type: FieldTypeNumber, DefaultValue: 20, Description: "Default number of items per page"},
{Key: "max_page_size", Label: "Max Page Size", Type: FieldTypeNumber, DefaultValue: 100, Description: "Maximum allowed page size"},
Description: "Validates and normalizes pagination query parameters (page, limit, offset)",
ConfigFields: []ConfigFieldDef{
{Key: "default_page", Label: "Default Page", Type: FieldTypeNumber, DefaultValue: 1, Description: "Default page number when none is provided"},
{Key: "default_limit", Label: "Default Limit", Type: FieldTypeNumber, DefaultValue: 20, Description: "Default number of items to return when no limit is provided"},
{Key: "max_limit", Label: "Max Limit", Type: FieldTypeNumber, DefaultValue: 100, Description: "Maximum allowed number of items to return per request"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. default_page_size/max_page_sizedefault_page, default_limit, max_limit matching the actual implementation.

Comment on lines +1727 to +1728
{Key: "queue", Label: "Queue Name", Type: FieldTypeString, Description: "DLQ queue name to replay from"},
{Key: "limit", Label: "Limit", Type: FieldTypeNumber, DefaultValue: 10, Description: "Maximum number of messages to replay"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.dlq_replay schema does not match the implementation. module/DLQReplayStep requires dlq_topic and target_topic and supports max_messages/broker; it does not use queue or limit. Please update the schema fields to match the step’s actual config keys.

Suggested change
{Key: "queue", Label: "Queue Name", Type: FieldTypeString, Description: "DLQ queue name to replay from"},
{Key: "limit", Label: "Limit", Type: FieldTypeNumber, DefaultValue: 10, Description: "Maximum number of messages to replay"},
{Key: "dlq_topic", Label: "DLQ Topic", Type: FieldTypeString, Description: "DLQ topic name to replay messages from"},
{Key: "target_topic", Label: "Target Topic", Type: FieldTypeString, Description: "Target topic to publish replayed messages to"},
{Key: "max_messages", Label: "Max Messages", Type: FieldTypeNumber, DefaultValue: 10, Description: "Maximum number of messages to replay"},
{Key: "broker", Label: "Broker", Type: FieldTypeString, Description: "Name of the messaging broker to use for replay"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. queue/limitdlq_topic + target_topic (both required), max_messages (default 100), and broker.

Comment on lines +1647 to +1649
{Key: "value_from", Label: "Value From", Type: FieldTypeString, Description: "Dotted path to the value to cache"},
{Key: "ttl", Label: "TTL", Type: FieldTypeDuration, Description: "Cache entry time-to-live", Placeholder: "5m"},
{Key: "cache", Label: "Cache Module", Type: FieldTypeString, Description: "Name of the cache module to use"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.cache_set schema uses value_from, but the implementation requires value (a template string) and requires cache. Please update the schema to use the correct key names and required fields to match module/CacheSetStep.

Suggested change
{Key: "value_from", Label: "Value From", Type: FieldTypeString, Description: "Dotted path to the value to cache"},
{Key: "ttl", Label: "TTL", Type: FieldTypeDuration, Description: "Cache entry time-to-live", Placeholder: "5m"},
{Key: "cache", Label: "Cache Module", Type: FieldTypeString, Description: "Name of the cache module to use"},
{Key: "value", Label: "Value", Type: FieldTypeString, Required: true, Description: "Value to cache (template string, supports expressions like {{.field}})"},
{Key: "ttl", Label: "TTL", Type: FieldTypeDuration, Description: "Cache entry time-to-live", Placeholder: "5m"},
{Key: "cache", Label: "Cache Module", Type: FieldTypeString, Required: true, Description: "Name of the cache module to use"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. value_fromvalue (required, supports template expressions), cache is now required.

Comment on lines +1670 to +1672
{Key: "event_type", Label: "Event Type", Type: FieldTypeString, Required: true, Description: "Event type identifier to publish", Placeholder: "user.created"},
{Key: "payload_from", Label: "Payload From", Type: FieldTypeString, Description: "Dotted path to the event payload in the pipeline context"},
{Key: "payload", Label: "Payload", Type: FieldTypeJSON, Description: "Static event payload (supports template expressions)"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.event_publish schema does not match the implementation: module/EventPublishStep requires topic (not event_type) and supports optional payload (map), headers (map), event_type, and broker. The schema currently documents event_type as required and includes payload_from, which the step does not read. Please align the schema with the actual config keys.

Suggested change
{Key: "event_type", Label: "Event Type", Type: FieldTypeString, Required: true, Description: "Event type identifier to publish", Placeholder: "user.created"},
{Key: "payload_from", Label: "Payload From", Type: FieldTypeString, Description: "Dotted path to the event payload in the pipeline context"},
{Key: "payload", Label: "Payload", Type: FieldTypeJSON, Description: "Static event payload (supports template expressions)"},
{Key: "topic", Label: "Topic", Type: FieldTypeString, Required: true, Description: "Topic or channel to publish the event to", Placeholder: "user-events"},
{Key: "payload", Label: "Payload", Type: FieldTypeJSON, Description: "Event payload as a JSON object (supports template expressions)"},
{Key: "headers", Label: "Headers", Type: FieldTypeJSON, Description: "Additional headers/metadata to include with the event as a JSON object"},
{Key: "event_type", Label: "Event Type", Type: FieldTypeString, Description: "Optional event type identifier to include with the message", Placeholder: "user.created"},
{Key: "broker", Label: "Broker", Type: FieldTypeString, Description: "Name of the messaging broker module to use"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. event_typetopic (required), removed payload_from, added headers, event_type (optional), and broker.

Comment on lines +1713 to +1717
Description: "Sends a failed message to the dead letter queue for later replay",
ConfigFields: []ConfigFieldDef{
{Key: "queue", Label: "Queue Name", Type: FieldTypeString, Description: "DLQ queue name (defaults to pipeline name)"},
{Key: "reason_from", Label: "Reason From", Type: FieldTypeString, Description: "Context key containing the failure reason"},
{Key: "payload_from", Label: "Payload From", Type: FieldTypeString, Description: "Dotted path to the message payload"},
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

The step.dlq_send schema does not match the implementation. module/DLQSendStep requires topic and supports optional original_topic, error, payload (map), and broker; it does not use queue, reason_from, or payload_from. Please align the schema fields with the actual step config keys.

Suggested change
Description: "Sends a failed message to the dead letter queue for later replay",
ConfigFields: []ConfigFieldDef{
{Key: "queue", Label: "Queue Name", Type: FieldTypeString, Description: "DLQ queue name (defaults to pipeline name)"},
{Key: "reason_from", Label: "Reason From", Type: FieldTypeString, Description: "Context key containing the failure reason"},
{Key: "payload_from", Label: "Payload From", Type: FieldTypeString, Description: "Dotted path to the message payload"},
Description: "Sends a failed message to the dead letter topic for later replay",
ConfigFields: []ConfigFieldDef{
{Key: "topic", Label: "DLQ Topic", Type: FieldTypeString, Required: true, Description: "Dead letter topic to publish failed messages to"},
{Key: "original_topic", Label: "Original Topic", Type: FieldTypeString, Description: "Optional name of the original topic the message came from"},
{Key: "error", Label: "Error", Type: FieldTypeString, Description: "Optional error message or context key containing the failure reason"},
{Key: "payload", Label: "Payload", Type: FieldTypeMap, Description: "Message payload to send to the DLQ"},
{Key: "broker", Label: "Broker", Type: FieldTypeString, Description: "Optional name of the messaging broker to use"},

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. queue/reason_from/payload_fromtopic (required), original_topic, error, payload, broker.

schema/schema.go Outdated
defer dynamicTriggerMu.RUnlock()

if len(dynamicTriggerTypes) == 0 {
return core
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

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

KnownTriggerTypes() docstring says the result is sorted and deduplicated, but when no dynamic trigger types are registered it returns the unsorted core slice as-is ("http", "schedule", "event", "eventbus"). This makes ordering inconsistent depending on whether plugins are present. Consider always returning a sorted copy (or calling sort.Strings(core) before returning) so callers get deterministic ordering.

Suggested change
return core
out := make([]string, len(core))
copy(out, core)
sort.Strings(out)
return out

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 42b6c99. Both KnownTriggerTypes and KnownWorkflowTypes now always return a sorted copy, even when no dynamic types are registered.

@intel352
Copy link
Contributor

@copilot apply changes based on the comments in this thread

…tions

- KnownTriggerTypes/KnownWorkflowTypes: always return sorted slice (even without dynamic types)
- step.webhook_verify: add both scheme-based and legacy provider-based fields
- step.cache_get: output_key→output (default "value"), cache required, add miss_ok
- step.cache_set: value_from→value (required), cache required
- step.cache_delete: cache now required
- step.event_publish: event_type→topic (required), remove payload_from, add headers/broker
- step.validate_pagination: default_page_size→default_page/default_limit, max_page_size→max_limit
- step.dlq_send: queue/reason_from/payload_from→topic (required)/original_topic/error/payload/broker
- step.dlq_replay: queue/limit→dlq_topic+target_topic (required)/max_messages/broker

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
@intel352 intel352 merged commit 74387e8 into main Feb 27, 2026
14 checks passed
@intel352 intel352 deleted the copilot/test-mcp-server-discoveries branch February 27, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants