diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml
index fdcad57a2f..608e365e00 100644
--- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml
+++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml
@@ -68,6 +68,57 @@ spec:
This embeds the CompositeToolConfig from pkg/vmcp/config to share the configuration model
between CLI and operator usage.
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
@@ -467,6 +518,57 @@ spec:
This embeds the CompositeToolConfig from pkg/vmcp/config to share the configuration model
between CLI and operator usage.
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml
index 11bd96ea31..ada9e5f512 100644
--- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml
+++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml
@@ -1077,24 +1077,34 @@ spec:
Only specified fields are overridden; others pass through from the backend.
properties:
destructiveHint:
- description: DestructiveHint overrides the
- destructive hint annotation.
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
idempotentHint:
- description: IdempotentHint overrides the
- idempotent hint annotation.
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
openWorldHint:
- description: OpenWorldHint overrides the open-world
- hint annotation.
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
readOnlyHint:
- description: ReadOnlyHint overrides the read-only
- hint annotation.
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
title:
- description: Title overrides the human-readable
- title annotation.
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: string
type: object
description:
@@ -1322,6 +1332,57 @@ spec:
CompositeToolConfig defines a composite tool workflow.
This matches the YAML structure from the proposal (lines 173-255).
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
@@ -4541,24 +4602,34 @@ spec:
Only specified fields are overridden; others pass through from the backend.
properties:
destructiveHint:
- description: DestructiveHint overrides the
- destructive hint annotation.
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
idempotentHint:
- description: IdempotentHint overrides the
- idempotent hint annotation.
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
openWorldHint:
- description: OpenWorldHint overrides the open-world
- hint annotation.
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
readOnlyHint:
- description: ReadOnlyHint overrides the read-only
- hint annotation.
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
title:
- description: Title overrides the human-readable
- title annotation.
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: string
type: object
description:
@@ -4786,6 +4857,57 @@ spec:
CompositeToolConfig defines a composite tool workflow.
This matches the YAML structure from the proposal (lines 173-255).
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml
index 8a7c7efc85..db2e16f117 100644
--- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml
+++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml
@@ -71,6 +71,57 @@ spec:
This embeds the CompositeToolConfig from pkg/vmcp/config to share the configuration model
between CLI and operator usage.
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
@@ -470,6 +521,57 @@ spec:
This embeds the CompositeToolConfig from pkg/vmcp/config to share the configuration model
between CLI and operator usage.
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml
index e8a5c6e084..c6d4a64509 100644
--- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml
+++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml
@@ -1080,24 +1080,34 @@ spec:
Only specified fields are overridden; others pass through from the backend.
properties:
destructiveHint:
- description: DestructiveHint overrides the
- destructive hint annotation.
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
idempotentHint:
- description: IdempotentHint overrides the
- idempotent hint annotation.
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
openWorldHint:
- description: OpenWorldHint overrides the open-world
- hint annotation.
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
readOnlyHint:
- description: ReadOnlyHint overrides the read-only
- hint annotation.
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
title:
- description: Title overrides the human-readable
- title annotation.
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: string
type: object
description:
@@ -1325,6 +1335,57 @@ spec:
CompositeToolConfig defines a composite tool workflow.
This matches the YAML structure from the proposal (lines 173-255).
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
@@ -4544,24 +4605,34 @@ spec:
Only specified fields are overridden; others pass through from the backend.
properties:
destructiveHint:
- description: DestructiveHint overrides the
- destructive hint annotation.
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
idempotentHint:
- description: IdempotentHint overrides the
- idempotent hint annotation.
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
openWorldHint:
- description: OpenWorldHint overrides the open-world
- hint annotation.
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
readOnlyHint:
- description: ReadOnlyHint overrides the read-only
- hint annotation.
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: boolean
title:
- description: Title overrides the human-readable
- title annotation.
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
type: string
type: object
description:
@@ -4789,6 +4860,57 @@ spec:
CompositeToolConfig defines a composite tool workflow.
This matches the YAML structure from the proposal (lines 173-255).
properties:
+ annotations:
+ description: |-
+ Annotations declares MCP tool annotations for the composite tool.
+
+ Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ and the backend tools are aggregated), not at CRD admission — thv vmcp
+ validate does NOT check annotation contradictions. The derived floor is
+ fail-closed: when the workflow has one or more tool steps the floor is
+ always non-nil, and any step whose annotations are nil/unknown taints the
+ floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ A workflow with no tool steps (e.g. only elicitation) has no floor.
+
+ When nil, annotations are derived from the annotations of the backend tools
+ referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ step tool is read-only). When set, the values are an explicit author
+ declaration merged over the derived floor — subject to a safety-floor
+ guardrail that drops the composite tool (with a warning naming the offending
+ step tools) if an explicit hint would make the tool look safer than its
+ steps allow.
+ properties:
+ destructiveHint:
+ description: |-
+ DestructiveHint sets the destructive hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ idempotentHint:
+ description: |-
+ IdempotentHint sets the idempotent hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ openWorldHint:
+ description: |-
+ OpenWorldHint sets the open-world hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ readOnlyHint:
+ description: |-
+ ReadOnlyHint sets the read-only hint annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: boolean
+ title:
+ description: |-
+ Title sets the human-readable title annotation.
+ For tool overrides this replaces the backend value; for composite tools
+ it is an explicit declaration merged over the derived safety floor.
+ type: string
+ type: object
description:
description: Description describes what the workflow does.
type: string
diff --git a/docs/arch/10-virtual-mcp-architecture.md b/docs/arch/10-virtual-mcp-architecture.md
index 766d138444..2b6fbe6cac 100644
--- a/docs/arch/10-virtual-mcp-architecture.md
+++ b/docs/arch/10-virtual-mcp-architecture.md
@@ -252,7 +252,9 @@ Steps can be of three types:
- **elicitation**: Request user input via MCP elicitation protocol
- **forEach**: Iterate over a collection from a previous step, executing an inner tool step per item with bounded parallelism
-**Implementation**: `pkg/vmcp/composer/`
+**Tool annotations**: Composite tools advertise MCP tool annotations computed at advertise time via a derive-then-merge ordering. First, a safety floor is derived from the annotations of the step tools (including `forEach` inner steps): `readOnlyHint` is the AND across steps, `destructiveHint` and `openWorldHint` are the OR across steps (unknown steps taint conservatively), and `idempotentHint` is never derived. Then any explicit `annotations` declared on the composite tool definition are merged over the floor, with explicitly set fields winning. An explicit hint may be more conservative than the floor, but if it would make the tool look safer than its steps allow (e.g. `readOnlyHint: true` when a step is not read-only), the composite tool is dropped from `tools/list` with a warning rather than advertised misleadingly.
+
+**Implementation**: `pkg/vmcp/composer/` (execution), `pkg/vmcp/internal/compositetools/` (advertised tool conversion and annotation derivation)
## Backend MCP Revision Classification
diff --git a/docs/operator/composite-tools-quick-reference.md b/docs/operator/composite-tools-quick-reference.md
index 719026a293..640350ee00 100644
--- a/docs/operator/composite-tools-quick-reference.md
+++ b/docs/operator/composite-tools-quick-reference.md
@@ -22,6 +22,23 @@ spec:
description: Description of the parameter
required: false
+ annotations: # Optional: explicit MCP tool annotations
+ title: My Workflow # Derivation runs at advertise time, not CRD
+ # admission. The floor is fail-closed: with
+ # one or more tool steps it is always non-nil,
+ # and any step with nil/unknown annotations
+ # taints it to readOnly=false,
+ # destructive=true, openWorld=true.
+ readOnlyHint: false # ⚠ An explicit readOnlyHint: true here would
+ # CONTRADICT the floor whenever a step tool
+ # does not declare readOnlyHint: true (the
+ # common case), dropping the composite tool
+ # from tools/list at runtime with a warning.
+ # thv vmcp validate does NOT catch this — it
+ # surfaces only in the vMCP logs. Prefer
+ # leaving readOnlyHint unset (derive it) or
+ # declaring a value no safer than the floor.
+
steps: # Required: workflow steps
- id: step1
type: tool # tool|elicitation|forEach
@@ -284,6 +301,7 @@ steps:
- ✅ forEach maxParallel: 50 (hard cap), defaults to DAG maxParallel (10)
- ✅ forEach inner step must be type `tool` (no nested forEach or elicitation)
- ✅ forEach `itemVar` cannot be `index` (reserved)
+- ✅ Annotations must not contradict the step-derived safety floor (e.g. `readOnlyHint: true` when a step is not read-only) — a contradicting composite tool is **dropped from `tools/list` at runtime** with a warning in the vMCP logs (more-conservative hints, e.g. `readOnlyHint: false`, are allowed)
**Note**: Max retry and max steps limits are currently enforced at runtime. Future work may add CRD-level validation (`+kubebuilder:validation:MaxItems=100`) and webhook validation to fail at submission time rather than execution time.
diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md
index 5b32a3cf92..ea165011c5 100644
--- a/docs/operator/crd-api.md
+++ b/docs/operator/crd-api.md
@@ -365,6 +365,7 @@ _Appears in:_
| `timeout` _[vmcp.config.Duration](#vmcpconfigduration)_ | Timeout is the maximum workflow execution time. | | Pattern: `^([0-9]+(\.[0-9]+)?(ns\|us\|µs\|ms\|s\|m\|h))+$`
Type: string
|
| `steps` _[vmcp.config.WorkflowStepConfig](#vmcpconfigworkflowstepconfig) array_ | Steps are the workflow steps to execute. | | |
| `output` _[vmcp.config.OutputConfig](#vmcpconfigoutputconfig)_ | Output defines the structured output schema for this workflow.
If not specified, the workflow returns the last step's output (backward compatible). | | Optional: \{\}
|
+| `annotations` _[vmcp.config.ToolAnnotationsOverride](#vmcpconfigtoolannotationsoverride)_ | Annotations declares MCP tool annotations for the composite tool.
Annotation derivation runs at ADVERTISE TIME (when tools/list is served
and the backend tools are aggregated), not at CRD admission — thv vmcp
validate does NOT check annotation contradictions. The derived floor is
fail-closed: when the workflow has one or more tool steps the floor is
always non-nil, and any step whose annotations are nil/unknown taints the
floor conservatively (readOnly=false, destructive=true, openWorld=true).
A workflow with no tool steps (e.g. only elicitation) has no floor.
When nil, annotations are derived from the annotations of the backend tools
referenced by the workflow's steps (e.g. readOnlyHint is true only when every
step tool is read-only). When set, the values are an explicit author
declaration merged over the derived floor — subject to a safety-floor
guardrail that drops the composite tool (with a warning naming the offending
step tools) if an explicit hint would make the tool look safer than its
steps allow. | | Optional: \{\}
|
#### vmcp.config.CompositeToolRef
@@ -760,7 +761,9 @@ All fields use pointers so nil means "don't override" while zero values
_Appears in:_
+- [vmcp.config.CompositeToolConfig](#vmcpconfigcompositetoolconfig)
- [vmcp.config.ToolOverride](#vmcpconfigtooloverride)
+- [api.v1beta1.VirtualMCPCompositeToolDefinitionSpec](#apiv1beta1virtualmcpcompositetooldefinitionspec)
@@ -4441,6 +4444,7 @@ _Appears in:_
| `timeout` _[vmcp.config.Duration](#vmcpconfigduration)_ | Timeout is the maximum workflow execution time. | | Pattern: `^([0-9]+(\.[0-9]+)?(ns\|us\|µs\|ms\|s\|m\|h))+$`
Type: string
|
| `steps` _[vmcp.config.WorkflowStepConfig](#vmcpconfigworkflowstepconfig) array_ | Steps are the workflow steps to execute. | | |
| `output` _[vmcp.config.OutputConfig](#vmcpconfigoutputconfig)_ | Output defines the structured output schema for this workflow.
If not specified, the workflow returns the last step's output (backward compatible). | | Optional: \{\}
|
+| `annotations` _[vmcp.config.ToolAnnotationsOverride](#vmcpconfigtoolannotationsoverride)_ | Annotations declares MCP tool annotations for the composite tool.
Annotation derivation runs at ADVERTISE TIME (when tools/list is served
and the backend tools are aggregated), not at CRD admission — thv vmcp
validate does NOT check annotation contradictions. The derived floor is
fail-closed: when the workflow has one or more tool steps the floor is
always non-nil, and any step whose annotations are nil/unknown taints the
floor conservatively (readOnly=false, destructive=true, openWorld=true).
A workflow with no tool steps (e.g. only elicitation) has no floor.
When nil, annotations are derived from the annotations of the backend tools
referenced by the workflow's steps (e.g. readOnlyHint is true only when every
step tool is read-only). When set, the values are an explicit author
declaration merged over the derived floor — subject to a safety-floor
guardrail that drops the composite tool (with a warning naming the offending
step tools) if an explicit hint would make the tool look safer than its
steps allow. | | Optional: \{\}
|
#### api.v1beta1.VirtualMCPCompositeToolDefinitionStatus
diff --git a/docs/operator/virtualmcpcompositetooldefinition-guide.md b/docs/operator/virtualmcpcompositetooldefinition-guide.md
index 14e47a4823..6921f823e7 100644
--- a/docs/operator/virtualmcpcompositetooldefinition-guide.md
+++ b/docs/operator/virtualmcpcompositetooldefinition-guide.md
@@ -347,6 +347,77 @@ spec:
- `abort`: Stop on first failure (default)
- `continue`: Execute all steps regardless of failures
+### Annotations
+
+Declare MCP tool annotations for the composite tool. Annotations are behavioral
+hints advertised to MCP clients in `tools/list` responses.
+
+```yaml
+spec:
+ name: user_report
+ description: Generate a read-only user report
+
+ annotations:
+ title: User Report
+ readOnlyHint: true
+
+ steps:
+ - id: get_user
+ tool: users.get
+```
+
+**Fields** (all optional; all hints are booleans):
+
+| Field | Type | Default | Semantics |
+|-------------------|--------|-------------------|-----------|
+| `title` | string | derived (none) | Human-readable title. Explicit value wins when set. |
+| `readOnlyHint` | bool | derived from steps | The composite tool does not modify its environment. |
+| `destructiveHint` | bool | derived from steps | The composite tool may perform destructive updates. |
+| `idempotentHint` | bool | none | Repeated calls with the same arguments have no additional effect. Never derived — only advertised when explicitly declared. |
+| `openWorldHint` | bool | derived from steps | The composite tool interacts with external entities. |
+
+**When `annotations` is omitted**, annotations are derived at advertise time
+from the annotations of the backend tools referenced by the workflow's steps
+(including `forEach` inner steps):
+
+- `readOnlyHint`: **AND** across steps — `true` only when every step tool
+ declares `readOnlyHint: true`; any step that is nil or `false` makes it `false`.
+- `destructiveHint`: **OR** across steps — `true` when any step declares it
+ `true`, or when any step's annotations are unknown.
+- `openWorldHint`: **OR** across steps — same rule as `destructiveHint`.
+- `idempotentHint`: never derived (left unset).
+
+> **Client compatibility note:** existing composite tools that previously
+> advertised no annotations now advertise the fail-closed floor
+> (`destructiveHint: true`, `openWorldHint: true`, `readOnlyHint: false`) when
+> they have tool steps. That matches the MCP spec defaults for omitted hints, so
+> conformant clients are unaffected — but clients that branch on
+> `annotations != nil` may start treating composites as destructive.
+
+**When `annotations` is set**, the explicit values are merged over the derived
+floor: each explicitly set field wins; unset fields keep the derived value.
+
+**Safety floor and contradiction guardrail**: an explicit hint may be *more
+conservative* than the derived floor (e.g. `readOnlyHint: false` when the floor
+is `true`), but it may never make the tool look *safer* than its steps allow
+(e.g. `readOnlyHint: true` when a step is not read-only, or
+`destructiveHint: false` when a step is destructive). The floor is
+**fail-closed**: when a workflow has one or more tool steps the floor is always
+non-nil, and any step whose annotations are nil or unknown taints it
+conservatively (`readOnly=false`, `destructive=true`, `openWorld=true`). In the
+common case where backends declare no annotations (e.g. yardstick echo), the
+floor is that conservative set, so an explicit `readOnlyHint: true` will
+contradict it and be dropped. This guardrail runs at **runtime (advertise
+time)**, not at admission: a contradicting composite tool is omitted from
+`tools/list` and is also uncallable via `tools/call` (with a warning naming the
+offending step tools in the vMCP logs), while the VirtualMCPServer stays Ready.
+
+> **`thv vmcp validate` does NOT check annotation contradictions.** A composite
+> tool definition that passes validation can still be dropped at runtime if its
+> explicit annotations contradict the derived safety floor — contradictions only
+> surface in the vMCP logs at advertise time. It cannot run at admission because
+> step-tool annotations are only known once backends are aggregated.
+
### Template Syntax
Use Go template syntax for dynamic values:
diff --git a/examples/operator/virtual-mcps/composite_tool_simple.yaml b/examples/operator/virtual-mcps/composite_tool_simple.yaml
index 7614787dc2..5f0ced1a6b 100644
--- a/examples/operator/virtual-mcps/composite_tool_simple.yaml
+++ b/examples/operator/virtual-mcps/composite_tool_simple.yaml
@@ -100,6 +100,20 @@ spec:
required:
- url
+ # Explicit MCP tool annotations advertised to clients. When omitted,
+ # annotations are derived from the step tools at advertise time. The floor
+ # is fail-closed: any step whose annotations are nil/unknown (gofetch
+ # fetch and yardstick echo declare none) taints it conservatively to
+ # readOnly=false, destructive=true, openWorld=true. Declaring
+ # readOnlyHint: true here would CONTRADICT that floor and cause the tool
+ # to be DROPPED from tools/list at runtime with a warning — so we declare
+ # openWorldHint: true (honest: fetch makes HTTP calls) and let derivation
+ # fill the rest. This guardrail runs at advertise time, not at CRD
+ # admission; thv vmcp validate does not catch it.
+ annotations:
+ title: Fetch and Validate Data
+ openWorldHint: true
+
# Sequential workflow steps
steps:
# Step 1: Fetch data from URL
diff --git a/examples/vmcp-config.yaml b/examples/vmcp-config.yaml
index 7b02dea7b7..438968ad3f 100644
--- a/examples/vmcp-config.yaml
+++ b/examples/vmcp-config.yaml
@@ -157,6 +157,18 @@ operational:
# required: ["pr_number"]
# timeout: "30m"
#
+# # Optional: explicit MCP tool annotations advertised to clients.
+# # When omitted, annotations are derived from the step tools at advertise
+# # time (readOnlyHint = AND across steps, destructiveHint/openWorldHint = OR,
+# # idempotentHint = never derived). Explicit values are merged over the
+# # derived floor but must never contradict it (e.g. readOnlyHint: true when
+# # a step is not read-only) — a contradicting composite tool is dropped
+# # from tools/list with a warning at runtime.
+# annotations:
+# title: "Deploy and Notify"
+# destructiveHint: true
+# openWorldHint: true
+#
# steps:
# - id: "merge"
# tool: "github.merge_pr"
diff --git a/pkg/vmcp/composer/composer.go b/pkg/vmcp/composer/composer.go
index 37b8ebc782..3771432474 100644
--- a/pkg/vmcp/composer/composer.go
+++ b/pkg/vmcp/composer/composer.go
@@ -69,6 +69,10 @@ type WorkflowDefinition struct {
// If nil, the workflow returns the last step's output (backward compatible).
Output *config.OutputConfig
+ // Annotations declares MCP tool annotations for the composite tool.
+ // If nil, annotations are derived from the step tools at advertise time.
+ Annotations *config.ToolAnnotationsOverride
+
// Metadata stores additional workflow information.
Metadata map[string]string
}
diff --git a/pkg/vmcp/config/annotations.go b/pkg/vmcp/config/annotations.go
new file mode 100644
index 0000000000..7b36905dda
--- /dev/null
+++ b/pkg/vmcp/config/annotations.go
@@ -0,0 +1,29 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package config
+
+import "github.com/stacklok/toolhive/pkg/vmcp"
+
+// ToAnnotations converts the wire-side override into the vmcp-domain
+// ToolAnnotations. It returns nil when o is nil. Pointer fields are shared
+// (callers that need a private copy should deep-copy the result).
+//
+// This lives in the config package (not in the compositetools derivation module)
+// so the pure derivation logic does not depend on the wire format — config owns
+// the mapping from its override type to the domain type it already imports.
+func (o *ToolAnnotationsOverride) ToAnnotations() *vmcp.ToolAnnotations {
+ if o == nil {
+ return nil
+ }
+ ann := &vmcp.ToolAnnotations{
+ ReadOnlyHint: o.ReadOnlyHint,
+ DestructiveHint: o.DestructiveHint,
+ IdempotentHint: o.IdempotentHint,
+ OpenWorldHint: o.OpenWorldHint,
+ }
+ if o.Title != nil {
+ ann.Title = *o.Title
+ }
+ return ann
+}
diff --git a/pkg/vmcp/config/annotations_test.go b/pkg/vmcp/config/annotations_test.go
new file mode 100644
index 0000000000..08c5df0451
--- /dev/null
+++ b/pkg/vmcp/config/annotations_test.go
@@ -0,0 +1,57 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package config
+
+import (
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func TestToolAnnotationsOverride_ToAnnotations(t *testing.T) {
+ t.Parallel()
+
+ t.Run("nil override returns nil", func(t *testing.T) {
+ t.Parallel()
+ var o *ToolAnnotationsOverride
+ assert.Nil(t, o.ToAnnotations())
+ })
+
+ t.Run("empty override returns non-nil with all-nil fields", func(t *testing.T) {
+ t.Parallel()
+ ann := (&ToolAnnotationsOverride{}).ToAnnotations()
+ require.NotNil(t, ann)
+ assert.Empty(t, ann.Title)
+ assert.Nil(t, ann.ReadOnlyHint)
+ assert.Nil(t, ann.DestructiveHint)
+ assert.Nil(t, ann.IdempotentHint)
+ assert.Nil(t, ann.OpenWorldHint)
+ })
+
+ t.Run("maps every field", func(t *testing.T) {
+ t.Parallel()
+ trueVal, falseVal := true, false
+ o := &ToolAnnotationsOverride{
+ Title: ptrToStr("My Tool"),
+ ReadOnlyHint: &trueVal,
+ DestructiveHint: &falseVal,
+ IdempotentHint: &trueVal,
+ OpenWorldHint: &falseVal,
+ }
+ ann := o.ToAnnotations()
+ require.NotNil(t, ann)
+ assert.Equal(t, "My Tool", ann.Title)
+ require.NotNil(t, ann.ReadOnlyHint)
+ assert.True(t, *ann.ReadOnlyHint)
+ require.NotNil(t, ann.DestructiveHint)
+ assert.False(t, *ann.DestructiveHint)
+ require.NotNil(t, ann.IdempotentHint)
+ assert.True(t, *ann.IdempotentHint)
+ require.NotNil(t, ann.OpenWorldHint)
+ assert.False(t, *ann.OpenWorldHint)
+ })
+}
+
+func ptrToStr(s string) *string { return &s }
diff --git a/pkg/vmcp/config/composite_validation.go b/pkg/vmcp/config/composite_validation.go
index 72f88d4384..a21fdad832 100644
--- a/pkg/vmcp/config/composite_validation.go
+++ b/pkg/vmcp/config/composite_validation.go
@@ -41,6 +41,13 @@ const (
// ValidateCompositeToolConfig validates a CompositeToolConfig.
// This is the primary entry point for composite tool validation, used by both
// webhooks (VirtualMCPServer, VirtualMCPCompositeToolDefinition) and runtime validation.
+//
+// Note: the tool.Annotations field requires no structural check here — all of
+// its fields are optional pointers with no invalid value. The annotations
+// safety-floor guardrail (rejecting explicit hints that would make the
+// composite tool look safer than its step tools allow) runs at runtime
+// (advertise time), not config-load, because step-tool annotations are only
+// known once backends are aggregated.
func ValidateCompositeToolConfig(pathPrefix string, tool *CompositeToolConfig) error {
var errors []string
diff --git a/pkg/vmcp/config/composite_validation_test.go b/pkg/vmcp/config/composite_validation_test.go
index f3e9a2c466..d94aff9039 100644
--- a/pkg/vmcp/config/composite_validation_test.go
+++ b/pkg/vmcp/config/composite_validation_test.go
@@ -12,6 +12,9 @@ import (
thvjson "github.com/stacklok/toolhive/pkg/json"
)
+// ptrTo returns a pointer to v. Used in tests for optional pointer fields.
+func ptrTo[T any](v T) *T { return &v }
+
func TestValidateDefaultResultsForSteps(t *testing.T) {
t.Parallel()
@@ -373,6 +376,24 @@ func TestValidateCompositeToolConfig(t *testing.T) {
expectError: true,
errorMsg: "name is required",
},
+ {
+ name: "annotations accepted without structural error",
+ tool: &CompositeToolConfig{
+ Name: "test-tool",
+ Description: "A test tool",
+ Steps: []WorkflowStepConfig{
+ {ID: "step1", Type: "tool", Tool: "backend.echo"},
+ },
+ Annotations: &ToolAnnotationsOverride{
+ Title: ptrTo("Test Tool"),
+ ReadOnlyHint: ptrTo(true),
+ DestructiveHint: ptrTo(false),
+ IdempotentHint: ptrTo(true),
+ OpenWorldHint: ptrTo(false),
+ },
+ },
+ expectError: false,
+ },
{
name: "missing description",
tool: &CompositeToolConfig{
diff --git a/pkg/vmcp/config/config.go b/pkg/vmcp/config/config.go
index bc31a7e745..1343df0282 100644
--- a/pkg/vmcp/config/config.go
+++ b/pkg/vmcp/config/config.go
@@ -541,23 +541,33 @@ type ToolConfigRef struct {
// +kubebuilder:object:generate=true
// +gendoc
type ToolAnnotationsOverride struct {
- // Title overrides the human-readable title annotation.
+ // Title sets the human-readable title annotation.
+ // For tool overrides this replaces the backend value; for composite tools
+ // it is an explicit declaration merged over the derived safety floor.
// +optional
Title *string `json:"title,omitempty" yaml:"title,omitempty"`
- // ReadOnlyHint overrides the read-only hint annotation.
+ // ReadOnlyHint sets the read-only hint annotation.
+ // For tool overrides this replaces the backend value; for composite tools
+ // it is an explicit declaration merged over the derived safety floor.
// +optional
ReadOnlyHint *bool `json:"readOnlyHint,omitempty" yaml:"readOnlyHint,omitempty"`
- // DestructiveHint overrides the destructive hint annotation.
+ // DestructiveHint sets the destructive hint annotation.
+ // For tool overrides this replaces the backend value; for composite tools
+ // it is an explicit declaration merged over the derived safety floor.
// +optional
DestructiveHint *bool `json:"destructiveHint,omitempty" yaml:"destructiveHint,omitempty"`
- // IdempotentHint overrides the idempotent hint annotation.
+ // IdempotentHint sets the idempotent hint annotation.
+ // For tool overrides this replaces the backend value; for composite tools
+ // it is an explicit declaration merged over the derived safety floor.
// +optional
IdempotentHint *bool `json:"idempotentHint,omitempty" yaml:"idempotentHint,omitempty"`
- // OpenWorldHint overrides the open-world hint annotation.
+ // OpenWorldHint sets the open-world hint annotation.
+ // For tool overrides this replaces the backend value; for composite tools
+ // it is an explicit declaration merged over the derived safety floor.
// +optional
OpenWorldHint *bool `json:"openWorldHint,omitempty" yaml:"openWorldHint,omitempty"`
}
@@ -720,6 +730,26 @@ type CompositeToolConfig struct {
// If not specified, the workflow returns the last step's output (backward compatible).
// +optional
Output *OutputConfig `json:"output,omitempty" yaml:"output,omitempty"`
+
+ // Annotations declares MCP tool annotations for the composite tool.
+ //
+ // Annotation derivation runs at ADVERTISE TIME (when tools/list is served
+ // and the backend tools are aggregated), not at CRD admission — thv vmcp
+ // validate does NOT check annotation contradictions. The derived floor is
+ // fail-closed: when the workflow has one or more tool steps the floor is
+ // always non-nil, and any step whose annotations are nil/unknown taints the
+ // floor conservatively (readOnly=false, destructive=true, openWorld=true).
+ // A workflow with no tool steps (e.g. only elicitation) has no floor.
+ //
+ // When nil, annotations are derived from the annotations of the backend tools
+ // referenced by the workflow's steps (e.g. readOnlyHint is true only when every
+ // step tool is read-only). When set, the values are an explicit author
+ // declaration merged over the derived floor — subject to a safety-floor
+ // guardrail that drops the composite tool (with a warning naming the offending
+ // step tools) if an explicit hint would make the tool look safer than its
+ // steps allow.
+ // +optional
+ Annotations *ToolAnnotationsOverride `json:"annotations,omitempty" yaml:"annotations,omitempty"`
}
// CompositeToolRef defines a reference to a VirtualMCPCompositeToolDefinition resource.
diff --git a/pkg/vmcp/config/crd_cli_roundtrip_test.go b/pkg/vmcp/config/crd_cli_roundtrip_test.go
index 1ad76d3064..30d12c9a81 100644
--- a/pkg/vmcp/config/crd_cli_roundtrip_test.go
+++ b/pkg/vmcp/config/crd_cli_roundtrip_test.go
@@ -489,6 +489,11 @@ func TestConfigRoundtrip(t *testing.T) {
Tool: "github-mcp.search_repos",
},
},
+ Annotations: &ToolAnnotationsOverride{
+ Title: ptrTo("Test Tool"),
+ ReadOnlyHint: ptrTo(true),
+ OpenWorldHint: ptrTo(true),
+ },
},
},
}
@@ -526,6 +531,24 @@ func TestConfigRoundtrip(t *testing.T) {
if parsedConfig.CompositeTools[0].Name != "test-tool" {
t.Errorf("CompositeTools[0].Name = %q, want %q", parsedConfig.CompositeTools[0].Name, "test-tool")
}
+ // Annotations must survive the YAML round-trip (guards the yaml tags).
+ wantAnn := originalConfig.CompositeTools[0].Annotations
+ gotAnn := parsedConfig.CompositeTools[0].Annotations
+ if gotAnn == nil {
+ t.Fatal("CompositeTools[0].Annotations is nil after round-trip")
+ }
+ if gotAnn.Title == nil || *gotAnn.Title != *wantAnn.Title {
+ t.Errorf("Annotations.Title = %v, want %v", gotAnn.Title, *wantAnn.Title)
+ }
+ if gotAnn.ReadOnlyHint == nil || *gotAnn.ReadOnlyHint != *wantAnn.ReadOnlyHint {
+ t.Errorf("Annotations.ReadOnlyHint = %v, want %v", gotAnn.ReadOnlyHint, *wantAnn.ReadOnlyHint)
+ }
+ if gotAnn.OpenWorldHint == nil || *gotAnn.OpenWorldHint != *wantAnn.OpenWorldHint {
+ t.Errorf("Annotations.OpenWorldHint = %v, want %v", gotAnn.OpenWorldHint, *wantAnn.OpenWorldHint)
+ }
+ if gotAnn.DestructiveHint != nil || gotAnn.IdempotentHint != nil {
+ t.Errorf("unset annotations hints = (%v, %v), want nil", gotAnn.DestructiveHint, gotAnn.IdempotentHint)
+ }
}
// containsString checks if s contains substr.
diff --git a/pkg/vmcp/config/validator_test.go b/pkg/vmcp/config/validator_test.go
index f0e03c243c..cf9d61f1c6 100644
--- a/pkg/vmcp/config/validator_test.go
+++ b/pkg/vmcp/config/validator_test.go
@@ -645,6 +645,28 @@ func TestValidator_ValidateCompositeTools(t *testing.T) {
},
wantErr: false,
},
+ {
+ name: "valid composite tool with annotations",
+ tools: []CompositeToolConfig{
+ {
+ Name: "report_workflow",
+ Description: "Read-only report workflow",
+ Timeout: Duration(30 * time.Minute),
+ Steps: []WorkflowStepConfig{
+ {
+ ID: "fetch",
+ Type: "tool",
+ Tool: "github.get_pr",
+ },
+ },
+ Annotations: &ToolAnnotationsOverride{
+ Title: ptrTo("Report Workflow"),
+ ReadOnlyHint: ptrTo(true),
+ },
+ },
+ },
+ wantErr: false,
+ },
{
name: "missing tool name",
tools: []CompositeToolConfig{
diff --git a/pkg/vmcp/config/zz_generated.deepcopy.go b/pkg/vmcp/config/zz_generated.deepcopy.go
index e460451b35..218e2e8bbe 100644
--- a/pkg/vmcp/config/zz_generated.deepcopy.go
+++ b/pkg/vmcp/config/zz_generated.deepcopy.go
@@ -124,6 +124,11 @@ func (in *CompositeToolConfig) DeepCopyInto(out *CompositeToolConfig) {
*out = new(OutputConfig)
(*in).DeepCopyInto(*out)
}
+ if in.Annotations != nil {
+ in, out := &in.Annotations, &out.Annotations
+ *out = new(ToolAnnotationsOverride)
+ (*in).DeepCopyInto(*out)
+ }
}
// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CompositeToolConfig.
diff --git a/pkg/vmcp/core/core_calls_test.go b/pkg/vmcp/core/core_calls_test.go
index 1e8e6185ce..fe69f63b81 100644
--- a/pkg/vmcp/core/core_calls_test.go
+++ b/pkg/vmcp/core/core_calls_test.go
@@ -15,6 +15,7 @@ import (
"github.com/stacklok/toolhive/pkg/vmcp"
"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
"github.com/stacklok/toolhive/pkg/vmcp/composer"
+ "github.com/stacklok/toolhive/pkg/vmcp/config"
)
// expectAggregation wires reg.List + agg.AggregateCapabilities to return agg once.
@@ -143,6 +144,51 @@ func TestCallTool_CompositeNotAccessible(t *testing.T) {
assert.ErrorIs(t, err, vmcp.ErrNotFound)
}
+// TestStepAnnotationResolver_DottedNameResolution (Q5) verifies that a
+// composite-tool step reference using the "{workloadID}.{toolName}" dot
+// convention resolves to the correct backend annotations through the routing
+// table's resolved (prefixed) key. With prefix conflict resolution the routing
+// table stores "be1_echo" while the step still references "be1.echo"; the
+// resolver must match via WorkloadID + original capability name and return the
+// annotations registered under the resolved name.
+func TestStepAnnotationResolver_DottedNameResolution(t *testing.T) {
+ t.Parallel()
+
+ trueVal, falseVal := true, false
+ backendAnn := &vmcp.ToolAnnotations{
+ ReadOnlyHint: &trueVal,
+ DestructiveHint: &falseVal,
+ OpenWorldHint: &falseVal,
+ }
+
+ agg := &aggregator.AggregatedCapabilities{
+ Tools: []vmcp.Tool{
+ {Name: "be1_echo", BackendID: testBackendID, Annotations: backendAnn},
+ },
+ RoutingTable: &vmcp.RoutingTable{Tools: map[string]*vmcp.BackendTarget{
+ "be1_echo": {
+ WorkloadID: testBackendID,
+ BaseURL: "http://" + testBackendID + ":8080",
+ OriginalCapabilityName: "echo",
+ },
+ }},
+ }
+
+ resolver := stepAnnotationResolver(agg)
+ require.NotNil(t, resolver)
+
+ // Dotted reference resolves to the prefixed routing-table key's annotations.
+ got := resolver("be1.echo")
+ require.NotNil(t, got, "dotted step ref must resolve through the routing table")
+ assert.Same(t, backendAnn, got, "resolved annotations must come from the resolved (prefixed) tool name")
+
+ // The resolved key itself also resolves (exact-match fast path).
+ assert.Same(t, backendAnn, resolver("be1_echo"))
+
+ // An unknown step tool resolves to nil (treated conservatively upstream).
+ assert.Nil(t, resolver("be1.unknown"))
+}
+
func TestReadResource(t *testing.T) {
t.Parallel()
cfg, m := baseConfig(t)
@@ -315,6 +361,102 @@ func TestCompositeNameConflict_AdvertisedEqualsExecuted(t *testing.T) {
assert.Equal(t, want, got, "conflicting name must resolve to the backend tool, not the composite")
}
+// TestCompositeAnnotationContradiction_AdvertisedEqualsExecuted is the parity
+// guard for annotation drop: a composite whose explicit annotations contradict
+// the derived floor is omitted from ListTools and must also be uncallable via
+// CallTool (ErrNotFound). No backend CallTool expectation is set — if the
+// composite incorrectly remains executable, CallTool would hit the composer and
+// invoke the backend, failing the mock.
+func TestCompositeAnnotationContradiction_AdvertisedEqualsExecuted(t *testing.T) {
+ t.Parallel()
+ cfg, m := baseConfig(t)
+ cfg.WorkflowDefs = map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ // Optimistic readOnlyHint against a silent backend (nil annotations →
+ // fail-closed floor with readOnly=false) is a contradiction → drop.
+ Annotations: &config.ToolAnnotationsOverride{ReadOnlyHint: boolPtr(true)},
+ Steps: []composer.WorkflowStep{{ID: "s1", Type: composer.StepTypeTool, Tool: "be1.echo"}},
+ },
+ }
+
+ beTarget := backendTarget()
+ // Backend tool has nil Annotations → silent backend → conservative floor.
+ agg := &aggregator.AggregatedCapabilities{
+ Tools: []vmcp.Tool{{Name: "be1.echo", BackendID: testBackendID}},
+ RoutingTable: &vmcp.RoutingTable{Tools: map[string]*vmcp.BackendTarget{
+ "be1.echo": beTarget,
+ }},
+ }
+ // Two aggregations: one for ListTools, one for CallTool.
+ m.reg.EXPECT().List(gomock.Any()).Return(nil).Times(2)
+ m.agg.EXPECT().AggregateCapabilities(gomock.Any(), gomock.Any()).Return(agg, nil).Times(2)
+
+ c, err := New(cfg)
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = c.Close() })
+
+ tools, err := c.ListTools(context.Background(), nil)
+ require.NoError(t, err)
+ require.Len(t, tools, 1)
+ assert.Equal(t, "be1.echo", tools[0].Name)
+ assert.Equal(t, testBackendID, tools[0].BackendID)
+
+ // No m.client.EXPECT().CallTool — a regression that executes the workflow
+ // would call the backend and fail the mock controller.
+ _, err = c.CallTool(context.Background(), nil, "wf", nil, nil)
+ assert.ErrorIs(t, err, vmcp.ErrNotFound,
+ "contradicting composite must be uncallable; advertised equals executed")
+}
+
+// TestCompositeNameConflict_WithOptimisticAnnotations_RoutesToBackend is the
+// bug-2 regression: a composite named like a backend tool and carrying
+// optimistic annotations (readOnlyHint:true) must still be detected as a name
+// conflict. Pre-fix, ConvertWorkflowDefsToTools + noop resolver dropped the
+// composite from conversion (annotation contradiction), so ValidateNoToolConflicts
+// never saw the name and CallTool executed the workflow. Post-fix,
+// CompositeToolNames feeds the conflict check → ALL composites dropped →
+// CallTool routes to the backend.
+func TestCompositeNameConflict_WithOptimisticAnnotations_RoutesToBackend(t *testing.T) {
+ t.Parallel()
+ cfg, m := baseConfig(t)
+ cfg.WorkflowDefs = map[string]*composer.WorkflowDefinition{
+ "be1.echo": {
+ Name: "be1.echo",
+ Annotations: &config.ToolAnnotationsOverride{ReadOnlyHint: boolPtr(true)},
+ Steps: []composer.WorkflowStep{{ID: "s1", Type: composer.StepTypeTool, Tool: "be1.echo"}},
+ },
+ }
+
+ beTarget := backendTarget()
+ agg := &aggregator.AggregatedCapabilities{
+ Tools: []vmcp.Tool{{Name: "be1.echo", BackendID: testBackendID}},
+ RoutingTable: &vmcp.RoutingTable{Tools: map[string]*vmcp.BackendTarget{
+ "be1.echo": beTarget,
+ }},
+ }
+ // Two aggregations: one for ListTools, one for CallTool.
+ m.reg.EXPECT().List(gomock.Any()).Return(nil).Times(2)
+ m.agg.EXPECT().AggregateCapabilities(gomock.Any(), gomock.Any()).Return(agg, nil).Times(2)
+
+ c, err := New(cfg)
+ require.NoError(t, err)
+ t.Cleanup(func() { _ = c.Close() })
+
+ tools, err := c.ListTools(context.Background(), nil)
+ require.NoError(t, err)
+ require.Len(t, tools, 1)
+ assert.Equal(t, "be1.echo", tools[0].Name)
+ assert.Equal(t, testBackendID, tools[0].BackendID)
+
+ want := &vmcp.ToolCallResult{StructuredContent: map[string]any{"from": "backend"}}
+ m.client.EXPECT().CallTool(gomock.Any(), beTarget, "be1.echo", gomock.Any(), gomock.Any(), gomock.Any()).Return(want, nil)
+
+ got, err := c.CallTool(context.Background(), nil, "be1.echo", nil, nil)
+ require.NoError(t, err)
+ assert.Equal(t, want, got, "conflicting composite with optimistic annotations must route to the backend")
+}
+
// TestComplete_RoutesPromptRef verifies a ref/prompt completion resolves the
// backend through the prompts routing table and forwards to the backend client.
func TestComplete_RoutesPromptRef(t *testing.T) {
diff --git a/pkg/vmcp/core/core_vmcp.go b/pkg/vmcp/core/core_vmcp.go
index 98c5523fca..0a3fcbfe39 100644
--- a/pkg/vmcp/core/core_vmcp.go
+++ b/pkg/vmcp/core/core_vmcp.go
@@ -638,7 +638,7 @@ func (c *coreVMCP) advertisedTools(agg *aggregator.AggregatedCapabilities) []vmc
return agg.Tools
}
- composite := compositetools.ConvertWorkflowDefsToTools(defs)
+ composite := compositetools.ConvertWorkflowDefsToTools(defs, stepAnnotationResolver(agg))
out := make([]vmcp.Tool, 0, len(agg.Tools)+len(composite))
out = append(out, agg.Tools...)
out = append(out, composite...)
@@ -647,9 +647,12 @@ func (c *coreVMCP) advertisedTools(agg *aggregator.AggregatedCapabilities) []vmc
// accessibleComposites returns the composite-tool definitions the core advertises
// (and therefore executes) for agg's view: those whose every tool step is reachable
-// in the routing table AND whose names do not collide with a backend tool. On a name
+// in the routing table, whose names do not collide with a backend tool, AND whose
+// explicit annotations do not contradict the derived safety floor. On a name
// collision ALL composites are dropped so the backend tool wins, matching the legacy
// compositeToolsDecorator (sessionmanager/factory.go:158-168, decorator.go:83-86).
+// On an annotation contradiction the offending composite is dropped (with a warning)
+// so CallTool and ListTools share the same set.
//
// This is the single source of truth shared by advertisedTools (what ListTools shows)
// and CallTool (what executes), so a withheld composite is never executed — advertised
@@ -665,11 +668,39 @@ func (c *coreVMCP) accessibleComposites(
if len(defs) == 0 {
return nil
}
- if err := compositetools.ValidateNoToolConflicts(agg.Tools, compositetools.ConvertWorkflowDefsToTools(defs)); err != nil {
+ // Name-only conflict check — must not run annotation conversion, which can
+ // drop composites carrying optimistic hints and hide a colliding name.
+ if err := compositetools.ValidateNoToolConflicts(
+ agg.Tools, compositetools.CompositeToolNames(defs)); err != nil {
slog.Warn("composite tool name conflict detected; omitting composite tools", "error", err)
return nil
}
- return defs
+ // Annotation guardrail: drop contradicting composites so CallTool and
+ // ListTools share the same set (advertised equals executed).
+ return compositetools.FilterWorkflowDefsByAnnotations(defs, stepAnnotationResolver(agg))
+}
+
+// stepAnnotationResolver returns a StepAnnotationResolver that maps a composite
+// tool's step reference ("{workloadID}.{toolName}") to the backend tool's
+// annotations, using agg's routing table and aggregated tools. Resolution goes
+// through the shared router.ResolveToolRef primitive (the same path
+// isToolStepAccessible uses), so accessibility filtering and annotation
+// resolution cannot drift. Unknown step tools resolve to nil annotations
+// (treated conservatively by the derivation).
+func stepAnnotationResolver(agg *aggregator.AggregatedCapabilities) compositetools.StepAnnotationResolver {
+ annByName := make(map[string]*vmcp.ToolAnnotations, len(agg.Tools))
+ for i := range agg.Tools {
+ if agg.Tools[i].Annotations != nil {
+ annByName[agg.Tools[i].Name] = agg.Tools[i].Annotations
+ }
+ }
+ return func(stepTool string) *vmcp.ToolAnnotations {
+ resolvedName, ok := router.ResolveToolRef(agg.RoutingTable, stepTool)
+ if !ok {
+ return nil
+ }
+ return annByName[resolvedName]
+ }
}
// validateConfig checks New's required inputs and the elicitation contract,
diff --git a/pkg/vmcp/internal/compositetools/annotations.go b/pkg/vmcp/internal/compositetools/annotations.go
new file mode 100644
index 0000000000..9a0e85ebc4
--- /dev/null
+++ b/pkg/vmcp/internal/compositetools/annotations.go
@@ -0,0 +1,158 @@
+// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
+package compositetools
+
+import (
+ "fmt"
+
+ "github.com/stacklok/toolhive/pkg/vmcp"
+)
+
+// StepAnnotationResolver resolves a composite-tool step's tool reference
+// ("{workloadID}.{toolName}") to the backend tool's annotations. It returns nil
+// when the step tool is unknown or the backend declares no annotations.
+type StepAnnotationResolver func(stepTool string) *vmcp.ToolAnnotations
+
+// DeriveCompositeAnnotations computes the safety-floor annotations for a
+// composite tool from the annotations of its step tools.
+//
+// Fail-closed contract (issue #6192):
+// - A workflow with NO tool steps (e.g. only elicitation steps) yields an
+// empty/nil stepAnn slice, and Derive returns nil — there is nothing to
+// derive, so no floor is advertised.
+// - A workflow with one or more tool steps yields a non-nil stepAnn slice,
+// and Derive ALWAYS returns a conservative floor, even when every step
+// tool's annotations are nil/unknown. An unknown step never makes the tool
+// look safer — it taints destructive/openWorld to true and readOnly to
+// false. This means an explicit readOnlyHint:true over backends that
+// declare no annotations (the common case) CONTRADICTS the floor and is
+// dropped by CheckAnnotationContradiction.
+//
+// Derivation rules per hint:
+// - ReadOnlyHint: AND across steps — true only when every step declares
+// readOnlyHint=true; any step that is nil or false makes it non-read-only.
+// - DestructiveHint: OR across steps — true when any step is nil or declares
+// destructiveHint=true (an unknown step is tainted conservatively).
+// - OpenWorldHint: OR across steps — true when any step is nil or declares
+// openWorldHint=true.
+// - IdempotentHint: never derived (always nil) — idempotency does not
+// compose across multi-step workflows.
+func DeriveCompositeAnnotations(stepAnn []*vmcp.ToolAnnotations) *vmcp.ToolAnnotations {
+ // No tool steps at all: nothing to derive. This is distinct from "≥1 tool
+ // step exists but none declares annotations", which produces a conservative
+ // floor below.
+ if len(stepAnn) == 0 {
+ return nil
+ }
+
+ readOnly := true
+ destructive := false
+ openWorld := false
+ for _, ann := range stepAnn {
+ if ann == nil {
+ readOnly = false
+ destructive = true
+ openWorld = true
+ continue
+ }
+ if ann.ReadOnlyHint == nil || !*ann.ReadOnlyHint {
+ readOnly = false
+ }
+ if ann.DestructiveHint == nil || *ann.DestructiveHint {
+ destructive = true
+ }
+ if ann.OpenWorldHint == nil || *ann.OpenWorldHint {
+ openWorld = true
+ }
+ }
+
+ return &vmcp.ToolAnnotations{
+ ReadOnlyHint: &readOnly,
+ DestructiveHint: &destructive,
+ OpenWorldHint: &openWorld,
+ }
+}
+
+// CheckAnnotationContradiction reports whether explicit annotations would make
+// a composite tool look SAFER than its derived safety floor allows:
+// - readOnlyHint=true while the floor is not true
+// - destructiveHint=false while the floor is true
+// - openWorldHint=false while the floor is true
+//
+// idempotentHint never contradicts, and an explicit hint MORE conservative than
+// the floor (e.g. readOnlyHint=false when the floor is true) is allowed.
+//
+// DeriveCompositeAnnotations always populates ReadOnlyHint, DestructiveHint,
+// and OpenWorldHint when it returns a non-nil floor, so the nil-hint branches
+// below are defensive (they would only matter for a hand-built floor). A nil
+// floor (no tool steps) or nil explicit means there is nothing to guard and
+// the check returns nil.
+func CheckAnnotationContradiction(explicit, floor *vmcp.ToolAnnotations) error {
+ if explicit == nil || floor == nil {
+ return nil
+ }
+
+ if explicit.ReadOnlyHint != nil && *explicit.ReadOnlyHint &&
+ (floor.ReadOnlyHint == nil || !*floor.ReadOnlyHint) {
+ return fmt.Errorf(
+ "annotations declare readOnlyHint=true, but not every step tool is read-only; " +
+ "the composite tool cannot be advertised as read-only")
+ }
+ if explicit.DestructiveHint != nil && !*explicit.DestructiveHint &&
+ floor.DestructiveHint != nil && *floor.DestructiveHint {
+ return fmt.Errorf(
+ "annotations declare destructiveHint=false, but at least one step tool is destructive or unknown; " +
+ "the composite tool cannot be advertised as non-destructive")
+ }
+ if explicit.OpenWorldHint != nil && !*explicit.OpenWorldHint &&
+ floor.OpenWorldHint != nil && *floor.OpenWorldHint {
+ return fmt.Errorf(
+ "annotations declare openWorldHint=false, but at least one step tool is open-world or unknown; " +
+ "the composite tool cannot be advertised as closed-world")
+ }
+ return nil
+}
+
+// MergeAnnotations merges explicit annotations over the derived floor. Per
+// hint, an explicit non-nil value wins over the floor; Title behaves the same
+// (a non-empty explicit Title wins). Returns nil when both inputs are nil.
+//
+// The returned value is a deep copy: *bool pointer fields and the Title string
+// are copied so the advertised tool does not alias the author's config
+// pointers. The inputs are not modified.
+func MergeAnnotations(floor, explicit *vmcp.ToolAnnotations) *vmcp.ToolAnnotations {
+ if floor == nil && explicit == nil {
+ return nil
+ }
+
+ merged := &vmcp.ToolAnnotations{}
+ if floor != nil {
+ copyBoolPtr(&merged.ReadOnlyHint, floor.ReadOnlyHint)
+ copyBoolPtr(&merged.DestructiveHint, floor.DestructiveHint)
+ copyBoolPtr(&merged.IdempotentHint, floor.IdempotentHint)
+ copyBoolPtr(&merged.OpenWorldHint, floor.OpenWorldHint)
+ merged.Title = floor.Title
+ }
+ if explicit != nil {
+ if explicit.Title != "" {
+ merged.Title = explicit.Title
+ }
+ copyBoolPtr(&merged.ReadOnlyHint, explicit.ReadOnlyHint)
+ copyBoolPtr(&merged.DestructiveHint, explicit.DestructiveHint)
+ copyBoolPtr(&merged.IdempotentHint, explicit.IdempotentHint)
+ copyBoolPtr(&merged.OpenWorldHint, explicit.OpenWorldHint)
+ }
+ return merged
+}
+
+// copyBoolPtr overwrites dst with a fresh copy of src when src is non-nil, so
+// dst does not alias src's memory. A nil src leaves dst untouched (preserving
+// the prior value, if any).
+func copyBoolPtr(dst **bool, src *bool) {
+ if src == nil {
+ return
+ }
+ v := *src
+ *dst = &v
+}
diff --git a/pkg/vmcp/internal/compositetools/workflow_converter.go b/pkg/vmcp/internal/compositetools/workflow_converter.go
index 1fb41f075d..a1290d81be 100644
--- a/pkg/vmcp/internal/compositetools/workflow_converter.go
+++ b/pkg/vmcp/internal/compositetools/workflow_converter.go
@@ -5,11 +5,12 @@ package compositetools
import (
"fmt"
- "strings"
+ "log/slog"
"github.com/stacklok/toolhive/pkg/vmcp"
"github.com/stacklok/toolhive/pkg/vmcp/composer"
"github.com/stacklok/toolhive/pkg/vmcp/config"
+ "github.com/stacklok/toolhive/pkg/vmcp/router"
)
// FilterWorkflowDefsForSession returns only the workflow definitions whose every
@@ -40,11 +41,6 @@ func FilterWorkflowDefsForSession(
// references a backend tool that is present in the session routing table.
// Returns false if rt is nil and the workflow contains any tool steps,
// since a nil routing table means no tools are routable in this session.
-//
-// Composite tool step names use the convention "{workloadID}.{toolName}" where
-// workloadID is a Kubernetes resource name (no dots). The routing table may store
-// tools under resolved/prefixed names (e.g. "{workloadID}_echo" with prefix strategy),
-// so we look up by BackendTarget.WorkloadID rather than the resolved key directly.
func allToolStepsAccessible(def *composer.WorkflowDefinition, rt *vmcp.RoutingTable) bool {
for _, step := range def.Steps {
if step.Type == composer.StepTypeTool {
@@ -71,38 +67,12 @@ func allToolStepsAccessible(def *composer.WorkflowDefinition, rt *vmcp.RoutingTa
}
// isToolStepAccessible reports whether a composite tool step's tool name can be
-// resolved to an accessible backend tool in the given routing table.
-//
-// Step tool names use the "{workloadID}.{toolName}" convention. Since conflict
-// resolution strategies (e.g. prefix) may rename tools in the routing table
-// (e.g. "echo" → "yardstick-backend_echo"), we check for accessibility by
-// matching on WorkloadID and the original backend capability name rather than
-// the resolved routing table key.
+// resolved to an accessible backend tool in the given routing table. It is a
+// thin nil-safe wrapper around the shared router.ResolveToolRef primitive so
+// accessibility filtering and annotation resolution cannot drift.
func isToolStepAccessible(stepTool string, rt *vmcp.RoutingTable) bool {
- // Fast path: exact match in the routing table.
- if _, ok := rt.Tools[stepTool]; ok {
- return true
- }
-
- // Parse "{workloadID}.{toolName}" convention.
- // Workload IDs are Kubernetes resource names and cannot contain dots,
- // so the first dot separates the workload ID from the tool name.
- dotIdx := strings.Index(stepTool, ".")
- if dotIdx <= 0 {
- return false
- }
- workloadID := stepTool[:dotIdx]
- originalName := stepTool[dotIdx+1:]
-
- for resolvedName, target := range rt.Tools {
- if target.WorkloadID != workloadID {
- continue
- }
- if target.GetBackendCapabilityName(resolvedName) == originalName {
- return true
- }
- }
- return false
+ _, ok := router.ResolveToolRef(rt, stepTool)
+ return ok
}
// ConvertWorkflowDefsToTools converts workflow definitions to vmcp.Tool format.
@@ -116,9 +86,24 @@ func isToolStepAccessible(stepTool string, rt *vmcp.RoutingTable) bool {
// - Description: workflow.Description
// - InputSchema: workflow.Parameters (JSON Schema format)
// - OutputSchema: workflow.Output (JSON Schema format, if defined)
+// - Annotations: the safety floor derived from the step tools' annotations,
+// merged with the workflow's explicit annotations, if any. When a workflow
+// has at least one tool step the floor is always non-nil (fail-closed):
+// backends that declare no annotations produce a conservative floor rather
+// than no floor.
+//
+// stepResolver may be nil (or return nil for every step), in which case each
+// tool step's annotations are treated as unknown and taint the floor
+// conservatively. When a workflow's explicit annotations contradict the derived
+// safety floor, the composite tool is DROPPED (not advertised) with a warning
+// that names the offending step tool(s) — an explicit declaration must never
+// make a tool look safer than its steps allow.
//
// Returns a slice of vmcp.Tool ready for aggregation and exposure to clients.
-func ConvertWorkflowDefsToTools(defs map[string]*composer.WorkflowDefinition) []vmcp.Tool {
+func ConvertWorkflowDefsToTools(
+ defs map[string]*composer.WorkflowDefinition,
+ stepResolver StepAnnotationResolver,
+) []vmcp.Tool {
if len(defs) == 0 {
return nil // Idiomatic Go: nil slice for empty result
}
@@ -136,12 +121,120 @@ func ConvertWorkflowDefsToTools(defs map[string]*composer.WorkflowDefinition) []
tool.OutputSchema = buildOutputSchema(def.Output)
}
+ // Derive the safety floor from the step tools, merge explicit
+ // annotations over it, and drop the tool on a contradiction.
+ if ann, ok := resolveCompositeAnnotations(def, stepResolver); ok {
+ tool.Annotations = ann
+ } else {
+ continue
+ }
+
tools = append(tools, tool)
}
return tools
}
+// FilterWorkflowDefsByAnnotations returns only the workflow definitions whose
+// explicit annotations do not contradict the derived safety floor. Definitions
+// that contradict are omitted (with the same warning resolveCompositeAnnotations
+// already emits). This is the CallTool/ListTools shared gate — callers that
+// execute composites must use this filtered set so a dropped tool is never callable.
+func FilterWorkflowDefsByAnnotations(
+ defs map[string]*composer.WorkflowDefinition,
+ stepResolver StepAnnotationResolver,
+) map[string]*composer.WorkflowDefinition {
+ if len(defs) == 0 {
+ return defs
+ }
+
+ filtered := make(map[string]*composer.WorkflowDefinition, len(defs))
+ for name, def := range defs {
+ if _, ok := resolveCompositeAnnotations(def, stepResolver); ok {
+ filtered[name] = def
+ }
+ }
+ return filtered
+}
+
+// resolveCompositeAnnotations computes the advertised annotations for a
+// composite tool: the floor derived from the step tools merged with the
+// workflow's explicit annotations. It returns ok=false when the explicit
+// annotations contradict the floor, in which case the caller must not
+// advertise the tool.
+//
+// stepRefs (the tool references of the steps that produced the floor) is
+// threaded into the drop warning so the author can locate the offending step.
+func resolveCompositeAnnotations(
+ def *composer.WorkflowDefinition,
+ stepResolver StepAnnotationResolver,
+) (ann *vmcp.ToolAnnotations, ok bool) {
+ // No resolver and no explicit annotations: nothing to derive or merge.
+ // (A workflow whose backends declare no annotations but has explicit
+ // annotations still runs through Derive + the contradiction guard below.)
+ if stepResolver == nil && def.Annotations == nil {
+ return nil, true
+ }
+
+ stepAnn, stepRefs := resolveStepAnnotations(def, stepResolver)
+ floor := DeriveCompositeAnnotations(stepAnn)
+ explicit := def.Annotations.ToAnnotations()
+ if err := CheckAnnotationContradiction(explicit, floor); err != nil {
+ slog.Warn("composite tool annotations contradict the safety floor; omitting composite tool",
+ "tool", def.Name, "step_tools", stepRefs, "error", err)
+ return nil, false
+ }
+ return MergeAnnotations(floor, explicit), true
+}
+
+// resolveStepAnnotations collects the annotations of every tool step in the
+// workflow, including forEach inner steps, mirroring the traversal in
+// allToolStepsAccessible. It returns the annotations and, in parallel, the
+// step tool references that produced them — so a contradiction warning can name
+// the offending step(s). A nil resolver (or an unknown step tool) yields a nil
+// entry, which the derivation treats conservatively as "unknown". A forEach
+// step with a nil InnerStep contributes nothing and never panics.
+func resolveStepAnnotations(
+ def *composer.WorkflowDefinition,
+ stepResolver StepAnnotationResolver,
+) (anns []*vmcp.ToolAnnotations, refs []string) {
+ resolve := func(stepTool string) {
+ var ann *vmcp.ToolAnnotations
+ if stepResolver != nil {
+ ann = stepResolver(stepTool)
+ }
+ anns = append(anns, ann)
+ refs = append(refs, stepTool)
+ }
+ for i := range def.Steps {
+ step := &def.Steps[i]
+ if step.Type == composer.StepTypeTool {
+ resolve(step.Tool)
+ }
+ // A forEach step whose InnerStep is nil is structurally invalid (caught
+ // earlier by validation); guard against a nil deref here regardless.
+ if step.Type == composer.StepTypeForEach && step.InnerStep != nil &&
+ step.InnerStep.Type == composer.StepTypeTool {
+ resolve(step.InnerStep.Tool)
+ }
+ }
+ return anns, refs
+}
+
+// CompositeToolNames returns the names of the composite tools in defs.
+// Order is undefined (map iteration). Used for name-conflict detection so
+// annotation policy cannot drop a colliding name from the conflict check.
+func CompositeToolNames(defs map[string]*composer.WorkflowDefinition) []string {
+ if len(defs) == 0 {
+ return nil
+ }
+ names := make([]string, 0, len(defs))
+ for name := range defs {
+ names = append(names, name)
+ }
+ return names
+}
+
// ValidateNoToolConflicts validates that composite tool names don't conflict with backend tool names.
//
// Tool name conflicts would cause ambiguity in routing/execution:
@@ -150,7 +243,10 @@ func ConvertWorkflowDefsToTools(defs map[string]*composer.WorkflowDefinition) []
//
// This validation ensures clear separation and prevents runtime confusion.
// Returns an error listing all conflicting tool names if any conflicts are found.
-func ValidateNoToolConflicts(backendTools, compositeTools []vmcp.Tool) error {
+//
+// Prefer CompositeToolNames(defs) for the compositeNames argument so conflict
+// detection never depends on annotation conversion (which can drop tools).
+func ValidateNoToolConflicts(backendTools []vmcp.Tool, compositeNames []string) error {
// Build set of backend tool names for O(1) lookups
backendNames := make(map[string]bool, len(backendTools))
for _, tool := range backendTools {
@@ -159,9 +255,9 @@ func ValidateNoToolConflicts(backendTools, compositeTools []vmcp.Tool) error {
// Check for conflicts
var conflicts []string
- for _, compTool := range compositeTools {
- if backendNames[compTool.Name] {
- conflicts = append(conflicts, compTool.Name)
+ for _, name := range compositeNames {
+ if backendNames[name] {
+ conflicts = append(conflicts, name)
}
}
diff --git a/pkg/vmcp/internal/compositetools/workflow_converter_test.go b/pkg/vmcp/internal/compositetools/workflow_converter_test.go
index 795a448c52..3fd64d18e3 100644
--- a/pkg/vmcp/internal/compositetools/workflow_converter_test.go
+++ b/pkg/vmcp/internal/compositetools/workflow_converter_test.go
@@ -4,9 +4,12 @@
package compositetools
import (
+ "errors"
"testing"
"github.com/google/go-cmp/cmp"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
"github.com/stacklok/toolhive/pkg/vmcp"
"github.com/stacklok/toolhive/pkg/vmcp/composer"
@@ -348,7 +351,7 @@ func TestConvertWorkflowDefsToToolsWithOutputSchema(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
- tools := ConvertWorkflowDefsToTools(tt.defs)
+ tools := ConvertWorkflowDefsToTools(tt.defs, nil)
if len(tools) != tt.want {
t.Errorf("ConvertWorkflowDefsToTools() returned %d tools, want %d", len(tools), tt.want)
@@ -536,6 +539,692 @@ func TestFilterWorkflowDefsForSession(t *testing.T) {
}
}
+func TestDeriveCompositeAnnotations(t *testing.T) {
+ t.Parallel()
+
+ trueVal, falseVal := true, false
+
+ tests := []struct {
+ name string
+ stepAnn []*vmcp.ToolAnnotations
+ want *vmcp.ToolAnnotations
+ }{
+ {
+ name: "empty input (no tool steps) yields nil floor",
+ stepAnn: nil,
+ want: nil,
+ },
+ {
+ // Fail-closed (issue #6192): ≥1 tool step exists but none declares
+ // annotations → conservative floor, NOT nil. An explicit readOnlyHint:true
+ // against this floor is dropped by CheckAnnotationContradiction.
+ name: "all nil annotations (tool steps exist, none declare) yields conservative floor",
+ stepAnn: []*vmcp.ToolAnnotations{nil, nil},
+ want: &vmcp.ToolAnnotations{
+ ReadOnlyHint: &falseVal,
+ DestructiveHint: &trueVal,
+ OpenWorldHint: &trueVal,
+ },
+ },
+ {
+ name: "all steps read-only",
+ stepAnn: []*vmcp.ToolAnnotations{
+ {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &trueVal},
+ },
+ want: &vmcp.ToolAnnotations{
+ ReadOnlyHint: &trueVal,
+ DestructiveHint: &falseVal,
+ OpenWorldHint: &trueVal,
+ },
+ },
+ {
+ name: "one non-read-only step makes floor not read-only",
+ stepAnn: []*vmcp.ToolAnnotations{
+ {ReadOnlyHint: &trueVal},
+ {ReadOnlyHint: &falseVal},
+ },
+ want: &vmcp.ToolAnnotations{
+ ReadOnlyHint: &falseVal,
+ DestructiveHint: &trueVal, // nil hints taint conservatively
+ OpenWorldHint: &trueVal,
+ },
+ },
+ {
+ name: "nil step annotations taint destructive and open-world",
+ stepAnn: []*vmcp.ToolAnnotations{
+ {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ nil,
+ },
+ want: &vmcp.ToolAnnotations{
+ ReadOnlyHint: &falseVal,
+ DestructiveHint: &trueVal,
+ OpenWorldHint: &trueVal,
+ },
+ },
+ {
+ name: "destructive OR across steps",
+ stepAnn: []*vmcp.ToolAnnotations{
+ {ReadOnlyHint: &falseVal, DestructiveHint: &trueVal, OpenWorldHint: &falseVal},
+ {ReadOnlyHint: &falseVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ },
+ want: &vmcp.ToolAnnotations{
+ ReadOnlyHint: &falseVal,
+ DestructiveHint: &trueVal,
+ OpenWorldHint: &falseVal,
+ },
+ },
+ {
+ name: "idempotent hint is never derived",
+ stepAnn: []*vmcp.ToolAnnotations{
+ {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal, IdempotentHint: &trueVal},
+ },
+ want: &vmcp.ToolAnnotations{
+ ReadOnlyHint: &trueVal,
+ DestructiveHint: &falseVal,
+ OpenWorldHint: &falseVal,
+ IdempotentHint: nil,
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ got := DeriveCompositeAnnotations(tt.stepAnn)
+
+ if diff := cmp.Diff(tt.want, got); diff != "" {
+ t.Errorf("DeriveCompositeAnnotations() mismatch (-want +got):\n%s", diff)
+ }
+ })
+ }
+}
+
+func TestCheckAnnotationContradiction(t *testing.T) {
+ t.Parallel()
+
+ trueVal, falseVal := true, false
+
+ tests := []struct {
+ name string
+ explicit *vmcp.ToolAnnotations
+ floor *vmcp.ToolAnnotations
+ wantErr bool
+ }{
+ {
+ name: "nil explicit",
+ explicit: nil,
+ floor: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ wantErr: false,
+ },
+ {
+ name: "nil floor",
+ explicit: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ floor: nil,
+ wantErr: false,
+ },
+ {
+ name: "readOnly true against non-read-only floor",
+ explicit: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ floor: &vmcp.ToolAnnotations{ReadOnlyHint: &falseVal},
+ wantErr: true,
+ },
+ {
+ name: "readOnly true against unknown read-only floor",
+ explicit: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ floor: &vmcp.ToolAnnotations{ReadOnlyHint: nil},
+ wantErr: true,
+ },
+ {
+ name: "destructive false against destructive floor",
+ explicit: &vmcp.ToolAnnotations{DestructiveHint: &falseVal},
+ floor: &vmcp.ToolAnnotations{DestructiveHint: &trueVal},
+ wantErr: true,
+ },
+ {
+ name: "openWorld false against open-world floor",
+ explicit: &vmcp.ToolAnnotations{OpenWorldHint: &falseVal},
+ floor: &vmcp.ToolAnnotations{OpenWorldHint: &trueVal},
+ wantErr: true,
+ },
+ {
+ name: "equal annotations",
+ explicit: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal, DestructiveHint: &falseVal},
+ floor: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal, DestructiveHint: &falseVal},
+ wantErr: false,
+ },
+ {
+ name: "more conservative readOnly false when floor is true",
+ explicit: &vmcp.ToolAnnotations{ReadOnlyHint: &falseVal},
+ floor: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ wantErr: false,
+ },
+ {
+ name: "more conservative destructive true when floor is false",
+ explicit: &vmcp.ToolAnnotations{DestructiveHint: &trueVal},
+ floor: &vmcp.ToolAnnotations{DestructiveHint: &falseVal},
+ wantErr: false,
+ },
+ {
+ name: "idempotent never contradicts",
+ explicit: &vmcp.ToolAnnotations{IdempotentHint: &trueVal},
+ floor: &vmcp.ToolAnnotations{IdempotentHint: &falseVal},
+ wantErr: false,
+ },
+ {
+ // Q1: explicit destructiveHint:false against a floor whose destructiveHint
+ // is nil (unknown) is allowed — the floor does not assert the tool is
+ // destructive, so the explicit claim does not contradict it.
+ name: "destructiveHint:false against nil-floor destructiveHint (nil-floor-hint guard)",
+ explicit: &vmcp.ToolAnnotations{DestructiveHint: &falseVal},
+ floor: &vmcp.ToolAnnotations{DestructiveHint: nil},
+ wantErr: false,
+ },
+ {
+ // Q2: explicit openWorldHint:false against a floor whose openWorldHint is
+ // nil (unknown) is allowed for the same reason.
+ name: "openWorldHint:false against nil-floor openWorldHint (nil-floor-hint guard)",
+ explicit: &vmcp.ToolAnnotations{OpenWorldHint: &falseVal},
+ floor: &vmcp.ToolAnnotations{OpenWorldHint: nil},
+ wantErr: false,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ err := CheckAnnotationContradiction(tt.explicit, tt.floor)
+
+ if (err != nil) != tt.wantErr {
+ t.Errorf("CheckAnnotationContradiction() error = %v, wantErr %v", err, tt.wantErr)
+ }
+ })
+ }
+}
+
+func TestMergeAnnotations(t *testing.T) {
+ t.Parallel()
+
+ trueVal, falseVal := true, false
+
+ tests := []struct {
+ name string
+ floor *vmcp.ToolAnnotations
+ explicit *vmcp.ToolAnnotations
+ want *vmcp.ToolAnnotations
+ }{
+ {
+ name: "both nil",
+ floor: nil,
+ explicit: nil,
+ want: nil,
+ },
+ {
+ name: "floor only",
+ floor: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ explicit: nil,
+ want: &vmcp.ToolAnnotations{ReadOnlyHint: &trueVal},
+ },
+ {
+ name: "explicit only",
+ floor: nil,
+ explicit: &vmcp.ToolAnnotations{Title: "T", IdempotentHint: &trueVal},
+ want: &vmcp.ToolAnnotations{Title: "T", IdempotentHint: &trueVal},
+ },
+ {
+ name: "explicit non-nil fields win per hint",
+ floor: &vmcp.ToolAnnotations{
+ Title: "Floor",
+ ReadOnlyHint: &trueVal,
+ DestructiveHint: &trueVal,
+ OpenWorldHint: &trueVal,
+ },
+ explicit: &vmcp.ToolAnnotations{
+ Title: "Explicit",
+ ReadOnlyHint: &falseVal,
+ IdempotentHint: &trueVal,
+ DestructiveHint: nil, // nil keeps floor
+ },
+ want: &vmcp.ToolAnnotations{
+ Title: "Explicit",
+ ReadOnlyHint: &falseVal,
+ DestructiveHint: &trueVal,
+ IdempotentHint: &trueVal,
+ OpenWorldHint: &trueVal,
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ got := MergeAnnotations(tt.floor, tt.explicit)
+
+ if diff := cmp.Diff(tt.want, got); diff != "" {
+ t.Errorf("MergeAnnotations() mismatch (-want +got):\n%s", diff)
+ }
+ })
+ }
+}
+
+func TestConvertWorkflowDefsToToolsAnnotations(t *testing.T) {
+ t.Parallel()
+
+ trueVal, falseVal := true, false
+
+ resolver := func(ann map[string]*vmcp.ToolAnnotations) StepAnnotationResolver {
+ return func(stepTool string) *vmcp.ToolAnnotations { return ann[stepTool] }
+ }
+
+ tests := []struct {
+ name string
+ defs map[string]*composer.WorkflowDefinition
+ stepResolver StepAnnotationResolver
+ want map[string]*vmcp.ToolAnnotations // tool name -> expected annotations (absent = dropped)
+ }{
+ {
+ name: "derives annotations from step tools",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ {ID: "s2", Type: composer.StepTypeTool, Tool: "backend.list"},
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ "backend.list": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ }),
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ },
+ },
+ {
+ name: "forEach inner step contributes to derivation",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ {
+ ID: "s2",
+ Type: composer.StepTypeForEach,
+ InnerStep: &composer.WorkflowStep{
+ ID: "inner", Type: composer.StepTypeTool, Tool: "backend.write",
+ },
+ },
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ "backend.write": {ReadOnlyHint: &falseVal, DestructiveHint: &trueVal, OpenWorldHint: &trueVal},
+ }),
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf": {ReadOnlyHint: &falseVal, DestructiveHint: &trueVal, OpenWorldHint: &trueVal},
+ },
+ },
+ {
+ name: "explicit annotations merge over the derived floor",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ },
+ Annotations: &config.ToolAnnotationsOverride{
+ Title: ptr("My Tool"),
+ IdempotentHint: &trueVal,
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ }),
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf": {
+ Title: "My Tool",
+ ReadOnlyHint: &trueVal,
+ DestructiveHint: &falseVal,
+ OpenWorldHint: &falseVal,
+ IdempotentHint: &trueVal,
+ },
+ },
+ },
+ {
+ name: "contradicting tool is dropped, others kept",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf_bad": {
+ Name: "wf_bad",
+ Description: "contradicting workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.write"},
+ },
+ Annotations: &config.ToolAnnotationsOverride{ReadOnlyHint: &trueVal},
+ },
+ "wf_ok": {
+ Name: "wf_ok",
+ Description: "valid workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ "backend.write": {ReadOnlyHint: &falseVal, DestructiveHint: &trueVal, OpenWorldHint: &trueVal},
+ }),
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf_ok": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ },
+ },
+ {
+ name: "unknown step tool taints the floor",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ {ID: "s2", Type: composer.StepTypeTool, Tool: "backend.unknown"},
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ }),
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf": {ReadOnlyHint: &falseVal, DestructiveHint: &trueVal, OpenWorldHint: &trueVal},
+ },
+ },
+ {
+ name: "nil resolver with no explicit annotations yields no annotations",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ },
+ },
+ },
+ stepResolver: nil,
+ want: map[string]*vmcp.ToolAnnotations{"wf": nil},
+ },
+ {
+ // Q4: with a nil stepResolver the floor cannot be derived (all steps are
+ // unknown), but an explicit annotation that does not tighten safety
+ // against the conservative floor still passes through.
+ name: "nil resolver with explicit annotations passes explicit through",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ },
+ Annotations: &config.ToolAnnotationsOverride{
+ Title: ptr("Explicit Tool"),
+ IdempotentHint: &trueVal,
+ },
+ },
+ },
+ stepResolver: nil,
+ // Conservative floor (readOnly=false, destructive=true, openWorld=true)
+ // merged with the explicit Title + IdempotentHint.
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf": {
+ Title: "Explicit Tool",
+ ReadOnlyHint: &falseVal,
+ DestructiveHint: &trueVal,
+ OpenWorldHint: &trueVal,
+ IdempotentHint: &trueVal,
+ },
+ },
+ },
+ {
+ // Q3: a forEach step with a nil InnerStep is structurally invalid but must
+ // be skipped (no panic) during annotation resolution.
+ name: "forEach with nil InnerStep is skipped without panic",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Description: "workflow",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ {ID: "s2", Type: composer.StepTypeForEach, InnerStep: nil},
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ }),
+ want: map[string]*vmcp.ToolAnnotations{
+ "wf": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ tools := ConvertWorkflowDefsToTools(tt.defs, tt.stepResolver)
+
+ if len(tools) != len(tt.want) {
+ t.Fatalf("ConvertWorkflowDefsToTools() returned %d tools, want %d", len(tools), len(tt.want))
+ }
+ for _, tool := range tools {
+ wantAnn, ok := tt.want[tool.Name]
+ if !ok {
+ t.Errorf("unexpected tool %q in result", tool.Name)
+ continue
+ }
+ if diff := cmp.Diff(wantAnn, tool.Annotations); diff != "" {
+ t.Errorf("tool %q annotations mismatch (-want +got):\n%s", tool.Name, diff)
+ }
+ }
+ })
+ }
+}
+
+func ptr[T any](v T) *T { return &v }
+
+func TestCompositeToolNames(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ defs map[string]*composer.WorkflowDefinition
+ want []string
+ }{
+ {
+ name: "nil map",
+ defs: nil,
+ want: nil,
+ },
+ {
+ name: "empty map",
+ defs: map[string]*composer.WorkflowDefinition{},
+ want: nil,
+ },
+ {
+ name: "returns all definition names",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf_a": {Name: "wf_a"},
+ "wf_b": {Name: "wf_b"},
+ "wf_c": {Name: "wf_c"},
+ },
+ want: []string{"wf_a", "wf_b", "wf_c"},
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+ got := CompositeToolNames(tt.defs)
+ require.ElementsMatch(t, tt.want, got)
+ })
+ }
+}
+
+func TestValidateNoToolConflicts(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ backendTools []vmcp.Tool
+ compositeNames []string
+ wantConflict bool
+ }{
+ {
+ name: "no conflict",
+ backendTools: []vmcp.Tool{{Name: "be1.echo"}, {Name: "be1.list"}},
+ compositeNames: []string{"wf", "deploy"},
+ wantConflict: false,
+ },
+ {
+ name: "single name conflict",
+ backendTools: []vmcp.Tool{{Name: "be1.echo"}, {Name: "shared"}},
+ compositeNames: []string{"wf", "shared"},
+ wantConflict: true,
+ },
+ {
+ name: "empty composites",
+ backendTools: []vmcp.Tool{{Name: "be1.echo"}},
+ compositeNames: nil,
+ wantConflict: false,
+ },
+ {
+ name: "empty backends",
+ backendTools: nil,
+ compositeNames: []string{"wf"},
+ wantConflict: false,
+ },
+ {
+ // Name-only check: even an optimistic/contradicting composite name is
+ // still visible to conflict detection (bug-2 regression surface).
+ name: "conflict with optimistic annotation name still detected",
+ backendTools: []vmcp.Tool{{Name: "be1.echo"}},
+ compositeNames: []string{"be1.echo"},
+ wantConflict: true,
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+ err := ValidateNoToolConflicts(tt.backendTools, tt.compositeNames)
+ if tt.wantConflict {
+ require.Error(t, err)
+ assert.True(t, errors.Is(err, vmcp.ErrToolNameConflict),
+ "expected ErrToolNameConflict, got %v", err)
+ return
+ }
+ require.NoError(t, err)
+ })
+ }
+}
+
+func TestFilterWorkflowDefsByAnnotations(t *testing.T) {
+ t.Parallel()
+
+ trueVal, falseVal := true, false
+ resolver := func(ann map[string]*vmcp.ToolAnnotations) StepAnnotationResolver {
+ return func(stepTool string) *vmcp.ToolAnnotations { return ann[stepTool] }
+ }
+
+ tests := []struct {
+ name string
+ defs map[string]*composer.WorkflowDefinition
+ stepResolver StepAnnotationResolver
+ wantNames []string
+ }{
+ {
+ name: "empty defs",
+ defs: map[string]*composer.WorkflowDefinition{},
+ wantNames: nil,
+ },
+ {
+ name: "keeps non-contradicting, drops contradicting",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf_bad": {
+ Name: "wf_bad",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.write"},
+ },
+ Annotations: &config.ToolAnnotationsOverride{ReadOnlyHint: &trueVal},
+ },
+ "wf_ok": {
+ Name: "wf_ok",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ "backend.write": {ReadOnlyHint: &falseVal, DestructiveHint: &trueVal, OpenWorldHint: &trueVal},
+ }),
+ wantNames: []string{"wf_ok"},
+ },
+ {
+ // Silent backend (nil step annotations) → fail-closed floor; explicit
+ // readOnlyHint:true contradicts and the def is dropped.
+ name: "drops optimistic readOnly against silent backend floor",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.echo"},
+ },
+ Annotations: &config.ToolAnnotationsOverride{ReadOnlyHint: &trueVal},
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.echo": nil,
+ }),
+ wantNames: []string{},
+ },
+ {
+ name: "keeps workflow with no explicit annotations",
+ defs: map[string]*composer.WorkflowDefinition{
+ "wf": {
+ Name: "wf",
+ Steps: []composer.WorkflowStep{
+ {ID: "s1", Type: composer.StepTypeTool, Tool: "backend.read"},
+ },
+ },
+ },
+ stepResolver: resolver(map[string]*vmcp.ToolAnnotations{
+ "backend.read": {ReadOnlyHint: &trueVal, DestructiveHint: &falseVal, OpenWorldHint: &falseVal},
+ }),
+ wantNames: []string{"wf"},
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ got := FilterWorkflowDefsByAnnotations(tt.defs, tt.stepResolver)
+ gotNames := make([]string, 0, len(got))
+ for name := range got {
+ gotNames = append(gotNames, name)
+ }
+ require.ElementsMatch(t, tt.wantNames, gotNames)
+ })
+ }
+}
+
func TestBuildOutputPropertySchema(t *testing.T) {
t.Parallel()
diff --git a/pkg/vmcp/router/session_router.go b/pkg/vmcp/router/session_router.go
index e7a9c970d2..b357807b5c 100644
--- a/pkg/vmcp/router/session_router.go
+++ b/pkg/vmcp/router/session_router.go
@@ -76,8 +76,45 @@ func compileResourceTemplates(rt *vmcp.RoutingTable) []compiledResourceTemplate
return compiled
}
+// ResolveToolRef resolves a composite-tool step reference ("{workloadID}.{toolName}")
+// to its routing-table key, returning ok=false when the reference does not
+// resolve. It is the shared primitive behind both composite-tool accessibility
+// filtering (compositetools.isToolStepAccessible) and per-step annotation
+// resolution (core.stepAnnotationResolver), so the two paths cannot drift.
+//
+// rt may be nil (returns "", false). Resolution order mirrors ResolveToolName:
+// 1. Exact key: the resolved/conflict-resolved name stored in rt.Tools.
+// 2. Dot convention "{workloadID}.{originalCapabilityName}": workload IDs
+// are Kubernetes resource names (no dots), so the first dot separates the
+// workload ID from the original backend capability name. A leading dot
+// (dotIdx == 0) is rejected so an empty workload ID never matches.
+func ResolveToolRef(rt *vmcp.RoutingTable, stepTool string) (resolvedName string, ok bool) {
+ if rt == nil || rt.Tools == nil || stepTool == "" {
+ return "", false
+ }
+
+ // Fast path: exact key match.
+ if _, exists := rt.Tools[stepTool]; exists {
+ return stepTool, true
+ }
+
+ // Fallback: dot convention "{workloadID}.{toolName}".
+ if dotIdx := strings.Index(stepTool, "."); dotIdx > 0 {
+ workloadID := stepTool[:dotIdx]
+ capName := stepTool[dotIdx+1:]
+ for resolvedName, target := range rt.Tools {
+ if target.WorkloadID == workloadID && target.GetBackendCapabilityName(resolvedName) == capName {
+ return resolvedName, true
+ }
+ }
+ }
+
+ return "", false
+}
+
// RouteTool resolves a tool name to its backend target using the session's
-// routing table directly.
+// routing table directly. Resolution is delegated to ResolveToolRef so this
+// path cannot drift from composite-tool accessibility filtering.
//
// Two naming conventions are supported:
//
@@ -94,59 +131,23 @@ func compileResourceTemplates(rt *vmcp.RoutingTable) []compiledResourceTemplate
// tools by their pre-conflict-resolution name (e.g. "my-backend.echo"), while
// the routing table may store them under a prefixed key ("my-backend_echo").
func (r *sessionRouter) RouteTool(_ context.Context, toolName string) (*vmcp.BackendTarget, error) {
- if r.routingTable == nil || r.routingTable.Tools == nil {
+ resolvedName, ok := ResolveToolRef(r.routingTable, toolName)
+ if !ok {
return nil, fmt.Errorf("%w: %s", ErrToolNotFound, toolName)
}
-
- // Fast path: exact key match.
- if target, exists := r.routingTable.Tools[toolName]; exists {
- return target, nil
- }
-
- // Fallback: dot convention "{workloadID}.{toolName}".
- // Workload IDs are Kubernetes resource names and cannot contain dots,
- // so the first dot unambiguously separates the workload ID from the
- // original backend capability name.
- if dotIdx := strings.Index(toolName, "."); dotIdx > 0 {
- workloadID := toolName[:dotIdx]
- capName := toolName[dotIdx+1:]
- for resolvedName, target := range r.routingTable.Tools {
- if target.WorkloadID == workloadID && target.GetBackendCapabilityName(resolvedName) == capName {
- return target, nil
- }
- }
- }
-
- return nil, fmt.Errorf("%w: %s", ErrToolNotFound, toolName)
+ return r.routingTable.Tools[resolvedName], nil
}
// ResolveToolName returns the routing table key (conflict-resolved name) for
-// toolName. If toolName is an exact key it is returned unchanged. If it uses
-// the dot convention "{workloadID}.{originalCapabilityName}", the matching
-// routing table key is returned. Falls back to returning toolName unchanged
-// when the routing table is absent or the name cannot be resolved (pass-through
-// semantics, consistent with the Router interface contract).
+// toolName via ResolveToolRef. If toolName is an exact key it is returned
+// unchanged. If it uses the dot convention "{workloadID}.{originalCapabilityName}",
+// the matching routing table key is returned. Falls back to returning toolName
+// unchanged when the routing table is absent or the name cannot be resolved
+// (pass-through semantics, consistent with the Router interface contract).
func (r *sessionRouter) ResolveToolName(_ context.Context, toolName string) string {
- if r.routingTable == nil || r.routingTable.Tools == nil {
- return toolName
- }
-
- // Fast path: exact key match.
- if _, exists := r.routingTable.Tools[toolName]; exists {
- return toolName
+ if n, ok := ResolveToolRef(r.routingTable, toolName); ok {
+ return n
}
-
- // Fallback: dot convention "{workloadID}.{toolName}".
- if dotIdx := strings.Index(toolName, "."); dotIdx > 0 {
- workloadID := toolName[:dotIdx]
- capName := toolName[dotIdx+1:]
- for resolvedName, target := range r.routingTable.Tools {
- if target.WorkloadID == workloadID && target.GetBackendCapabilityName(resolvedName) == capName {
- return resolvedName
- }
- }
- }
-
return toolName
}
diff --git a/pkg/vmcp/server/workflow_converter.go b/pkg/vmcp/server/workflow_converter.go
index edbebde9b5..36a13200c1 100644
--- a/pkg/vmcp/server/workflow_converter.go
+++ b/pkg/vmcp/server/workflow_converter.go
@@ -71,6 +71,7 @@ func ConvertConfigToWorkflowDefinitions(
Steps: steps,
Timeout: timeout,
Output: ct.Output,
+ Annotations: ct.Annotations,
Metadata: make(map[string]string),
}
diff --git a/pkg/vmcp/server/workflow_converter_test.go b/pkg/vmcp/server/workflow_converter_test.go
index 5dcaaa3285..8301e21fe4 100644
--- a/pkg/vmcp/server/workflow_converter_test.go
+++ b/pkg/vmcp/server/workflow_converter_test.go
@@ -297,3 +297,80 @@ func TestConvertConfigToWorkflowDefinitions_WithOutputConfig(t *testing.T) {
})
}
}
+
+// TestConvertConfigToWorkflowDefinitions_WithAnnotations tests that the
+// Annotations field is correctly copied from CompositeToolConfig to
+// WorkflowDefinition.
+func TestConvertConfigToWorkflowDefinitions_WithAnnotations(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ input []config.CompositeToolConfig
+ verify func(t *testing.T, defs map[string]*composer.WorkflowDefinition)
+ }{
+ {
+ name: "composite tool with annotations",
+ input: []config.CompositeToolConfig{
+ {
+ Name: "report_tool",
+ Description: "Read-only report",
+ Steps: []config.WorkflowStepConfig{
+ {ID: "fetch", Type: "tool", Tool: "data.fetch"},
+ },
+ Annotations: &config.ToolAnnotationsOverride{
+ Title: ptrTo("Report Tool"),
+ ReadOnlyHint: ptrTo(true),
+ },
+ },
+ },
+ verify: func(t *testing.T, defs map[string]*composer.WorkflowDefinition) {
+ t.Helper()
+ require.Len(t, defs, 1)
+
+ def, exists := defs["report_tool"]
+ require.True(t, exists)
+ require.NotNil(t, def.Annotations, "Annotations should be set on WorkflowDefinition")
+ require.NotNil(t, def.Annotations.Title)
+ assert.Equal(t, "Report Tool", *def.Annotations.Title)
+ require.NotNil(t, def.Annotations.ReadOnlyHint)
+ assert.True(t, *def.Annotations.ReadOnlyHint)
+ assert.Nil(t, def.Annotations.DestructiveHint)
+ },
+ },
+ {
+ name: "composite tool without annotations",
+ input: []config.CompositeToolConfig{
+ {
+ Name: "plain_tool",
+ Steps: []config.WorkflowStepConfig{{ID: "s1", Type: "tool", Tool: "data.fetch"}},
+ },
+ },
+ verify: func(t *testing.T, defs map[string]*composer.WorkflowDefinition) {
+ t.Helper()
+ require.Len(t, defs, 1)
+
+ def, exists := defs["plain_tool"]
+ require.True(t, exists)
+ assert.Nil(t, def.Annotations, "Annotations should be nil when unset (derive at advertise time)")
+ },
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+
+ result, err := ConvertConfigToWorkflowDefinitions(tt.input)
+ require.NoError(t, err)
+ require.NotNil(t, result)
+
+ if tt.verify != nil {
+ tt.verify(t, result)
+ }
+ })
+ }
+}
+
+// ptrTo returns a pointer to v. Used in tests for optional pointer fields.
+func ptrTo[T any](v T) *T { return &v }
diff --git a/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_referenced_test.go b/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_referenced_test.go
index 1860e1f7aa..f568c9168a 100644
--- a/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_referenced_test.go
+++ b/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_referenced_test.go
@@ -84,6 +84,13 @@ var _ = Describe("VirtualMCPServer Composite Referenced Workflow", Ordered, func
},
},
Timeout: vmcpconfig.Duration(30 * time.Second),
+ // Explicit annotations on the referenced definition. More
+ // conservative than any read-only floor, so they pass the
+ // safety-floor guardrail and reach ListTools.
+ Annotations: &vmcpconfig.ToolAnnotationsOverride{
+ Title: stringPtr("Referenced Echo Twice"),
+ DestructiveHint: boolPtr(true),
+ },
},
},
}
@@ -230,6 +237,36 @@ var _ = Describe("VirtualMCPServer Composite Referenced Workflow", Ordered, func
// Second echo: echoes the result of first echo
GinkgoWriter.Printf("Referenced composite tool result: %+v\n", result.Content)
})
+
+ It("should advertise the referenced definition's annotations in tool listing", func() {
+ By("Creating and initializing MCP client for VirtualMCPServer")
+ mcpClient, err := CreateInitializedMCPClient(vmcpNodePort, "toolhive-composite-ref-test", 30*time.Second)
+ Expect(err).ToNot(HaveOccurred())
+ defer mcpClient.Close()
+
+ By("Listing tools from VirtualMCPServer")
+ listRequest := mcp.ListToolsRequest{}
+ tools, err := mcpClient.Client.ListTools(mcpClient.Ctx, listRequest)
+ Expect(err).ToNot(HaveOccurred())
+
+ By("Verifying the referenced composite tool carries its explicit annotations")
+ var foundComposite bool
+ for _, tool := range tools.Tools {
+ if tool.Name == compositeToolName {
+ foundComposite = true
+ GinkgoWriter.Printf(" Tool: %s annotations=%+v\n", tool.Name, tool.Annotations)
+ Expect(tool.Annotations.Title).To(Equal("Referenced Echo Twice"))
+ Expect(tool.Annotations.DestructiveHint).ToNot(BeNil())
+ Expect(*tool.Annotations.DestructiveHint).To(BeTrue())
+ // The yardstick echo backend does not declare readOnlyHint,
+ // so the derived floor is not read-only.
+ Expect(tool.Annotations.ReadOnlyHint).ToNot(BeNil())
+ Expect(*tool.Annotations.ReadOnlyHint).To(BeFalse())
+ break
+ }
+ }
+ Expect(foundComposite).To(BeTrue(), "Should find referenced composite tool: %s", compositeToolName)
+ })
})
Context("when verifying referenced composite tool configuration", func() {
diff --git a/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_sequential_test.go b/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_sequential_test.go
index 67e74e3729..e6d11fd242 100644
--- a/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_sequential_test.go
+++ b/test/e2e/thv-operator/virtualmcp/virtualmcp_composite_sequential_test.go
@@ -31,7 +31,9 @@ var _ = Describe("VirtualMCPServer Composite Sequential Workflow", Ordered, func
vmcpNodePort int32
// Composite tool names
- compositeToolName = "echo_twice"
+ compositeToolName = "echo_twice"
+ annotatedToolName = "echo_annotated"
+ contradictingToolName = "echo_contradicting"
)
BeforeAll(func() {
@@ -87,6 +89,72 @@ var _ = Describe("VirtualMCPServer Composite Sequential Workflow", Ordered, func
},
},
},
+ // Composite tool with explicit annotations. An explicit hint may be
+ // MORE conservative than the derived floor, so this passes through.
+ {
+ Name: annotatedToolName,
+ Description: "Echoes the input with explicit conservative annotations",
+ Parameters: thvjson.NewMap(map[string]any{
+ "type": "object",
+ "properties": map[string]any{
+ "message": map[string]any{
+ "type": "string",
+ "description": "The message to echo",
+ },
+ },
+ "required": []any{"message"},
+ }),
+ Annotations: &vmcpconfig.ToolAnnotationsOverride{
+ Title: stringPtr("Annotated Echo"),
+ ReadOnlyHint: boolPtr(false),
+ DestructiveHint: boolPtr(true),
+ IdempotentHint: boolPtr(true),
+ },
+ Timeout: vmcpconfig.Duration(30 * time.Second),
+ Steps: []vmcpconfig.WorkflowStepConfig{
+ {
+ ID: "echo",
+ Type: "tool",
+ Tool: fmt.Sprintf("%s.echo", backendName),
+ Arguments: thvjson.NewMap(map[string]any{
+ "input": "{{ .params.message }}",
+ }),
+ },
+ },
+ },
+ // Composite tool whose explicit annotations CONTRADICT the derived
+ // safety floor: readOnlyHint=true claims the tool does not modify
+ // its environment, but the yardstick echo tool does not declare
+ // readOnlyHint, so the floor is not read-only. The guardrail drops
+ // this tool from tools/list at runtime while the server stays Ready.
+ {
+ Name: contradictingToolName,
+ Description: "Contradicting annotations: dropped by the safety-floor guardrail",
+ Parameters: thvjson.NewMap(map[string]any{
+ "type": "object",
+ "properties": map[string]any{
+ "message": map[string]any{
+ "type": "string",
+ "description": "The message to echo",
+ },
+ },
+ "required": []any{"message"},
+ }),
+ Annotations: &vmcpconfig.ToolAnnotationsOverride{
+ ReadOnlyHint: boolPtr(true),
+ },
+ Timeout: vmcpconfig.Duration(30 * time.Second),
+ Steps: []vmcpconfig.WorkflowStepConfig{
+ {
+ ID: "echo",
+ Type: "tool",
+ Tool: fmt.Sprintf("%s.echo", backendName),
+ Arguments: thvjson.NewMap(map[string]any{
+ "input": "{{ .params.message }}",
+ }),
+ },
+ },
+ },
},
}),
v1beta1test.WithVMCPIncomingAuth(&mcpv1beta1.IncomingAuthConfig{
@@ -195,6 +263,77 @@ var _ = Describe("VirtualMCPServer Composite Sequential Workflow", Ordered, func
// Second echo: echoes the result of first echo
GinkgoWriter.Printf("Composite tool result: %+v\n", result.Content)
})
+
+ It("should advertise annotations on composite tools", func() {
+ By("Creating and initializing MCP client for VirtualMCPServer")
+ mcpClient, err := CreateInitializedMCPClient(vmcpNodePort, "toolhive-composite-test", 30*time.Second)
+ Expect(err).ToNot(HaveOccurred())
+ defer mcpClient.Close()
+
+ By("Listing tools from VirtualMCPServer")
+ listRequest := mcp.ListToolsRequest{}
+ tools, err := mcpClient.Client.ListTools(mcpClient.Ctx, listRequest)
+ Expect(err).ToNot(HaveOccurred())
+
+ toolAnnotations := make(map[string]mcp.ToolAnnotation, len(tools.Tools))
+ for _, tool := range tools.Tools {
+ toolAnnotations[tool.Name] = tool.Annotations
+ GinkgoWriter.Printf(" Tool: %s annotations=%+v\n", tool.Name, tool.Annotations)
+ }
+
+ By("Verifying derived annotations on the composite tool")
+ // The yardstick echo backend declares NO annotations, so derivation is
+ // fail-closed: echo_twice (no explicit annotations) advertises the
+ // conservative floor — readOnlyHint=false, destructiveHint=true,
+ // openWorldHint=true.
+ compositeAnn, found := toolAnnotations[compositeToolName]
+ Expect(found).To(BeTrue(), "Should find composite tool: %s", compositeToolName)
+ Expect(compositeAnn.ReadOnlyHint).ToNot(BeNil(), "Composite tool should advertise a derived readOnlyHint")
+ Expect(*compositeAnn.ReadOnlyHint).To(BeFalse(),
+ "Derived readOnlyHint should be false: the yardstick echo tool does not declare itself read-only")
+ Expect(compositeAnn.DestructiveHint).ToNot(BeNil(), "Composite tool should advertise a derived destructiveHint")
+ Expect(*compositeAnn.DestructiveHint).To(BeTrue(),
+ "Derived destructiveHint should be true: a step whose annotations are unknown taints the floor")
+ Expect(compositeAnn.OpenWorldHint).ToNot(BeNil(), "Composite tool should advertise a derived openWorldHint")
+ Expect(*compositeAnn.OpenWorldHint).To(BeTrue(),
+ "Derived openWorldHint should be true: a step whose annotations are unknown taints the floor")
+
+ By("Verifying explicit annotations pass through when more conservative than the floor")
+ annotatedAnn, found := toolAnnotations[annotatedToolName]
+ Expect(found).To(BeTrue(), "Should find explicitly annotated composite tool: %s", annotatedToolName)
+ Expect(annotatedAnn.Title).To(Equal("Annotated Echo"))
+ Expect(annotatedAnn.ReadOnlyHint).ToNot(BeNil())
+ Expect(*annotatedAnn.ReadOnlyHint).To(BeFalse())
+ Expect(annotatedAnn.DestructiveHint).ToNot(BeNil())
+ Expect(*annotatedAnn.DestructiveHint).To(BeTrue())
+ Expect(annotatedAnn.IdempotentHint).ToNot(BeNil())
+ Expect(*annotatedAnn.IdempotentHint).To(BeTrue())
+
+ By("Verifying the contradicting composite tool is dropped")
+ _, found = toolAnnotations[contradictingToolName]
+ Expect(found).To(BeFalse(),
+ "Composite tool with annotations contradicting the safety floor should be dropped: %s", contradictingToolName)
+
+ By("Verifying the contradicting composite tool is also uncallable")
+ callRequest := mcp.CallToolRequest{}
+ callRequest.Params.Name = contradictingToolName
+ callRequest.Params.Arguments = map[string]any{"message": "should-not-run"}
+ _, err = mcpClient.Client.CallTool(mcpClient.Ctx, callRequest)
+ Expect(err).To(HaveOccurred(),
+ "CallTool on a dropped contradicting composite must fail; advertised equals executed")
+ })
+
+ It("should stay Ready while dropping the contradicting composite tool", func() {
+ vmcpServer := &mcpv1beta1.VirtualMCPServer{}
+ err := k8sClient.Get(ctx, types.NamespacedName{
+ Name: vmcpServerName,
+ Namespace: testNamespace,
+ }, vmcpServer)
+ Expect(err).ToNot(HaveOccurred())
+
+ Expect(vmcpServer.Status.Phase).To(Equal(mcpv1beta1.VirtualMCPServerPhaseReady),
+ "VirtualMCPServer should stay Ready when the annotation guardrail drops a composite tool")
+ })
})
Context("when verifying composite tool configuration", func() {
@@ -206,7 +345,7 @@ var _ = Describe("VirtualMCPServer Composite Sequential Workflow", Ordered, func
}, vmcpServer)
Expect(err).ToNot(HaveOccurred())
- Expect(vmcpServer.Spec.Config.CompositeTools).To(HaveLen(1))
+ Expect(vmcpServer.Spec.Config.CompositeTools).ToNot(BeEmpty())
compositeTool := vmcpServer.Spec.Config.CompositeTools[0]
Expect(compositeTool.Name).To(Equal(compositeToolName))
@@ -230,3 +369,9 @@ var _ = Describe("VirtualMCPServer Composite Sequential Workflow", Ordered, func
})
})
})
+
+// boolPtr returns a pointer to b. Used for optional *bool annotation fields.
+func boolPtr(b bool) *bool { return &b }
+
+// stringPtr returns a pointer to s. Used for optional *string annotation fields.
+func stringPtr(s string) *string { return &s }