Detect conflicting names for commands in metadata resolve step (#1168)

### What

Previously, MBS would not complain if two commands had the same root
field name, and would instead just keep whichever was resolved last.
This has now been fixed.

### How

We do precisely what we do with all the other steps: check a running
list of graphql names that have already been used. We've added a test
that does indeed fail on `main`, which now works.

V3_GIT_ORIGIN_REV_ID: fdf8f73acc62abf315125636c0010b01f6cdb96b

v3/changelog.md

@@ -38,6 +38,10 @@ query MyQuery
 - OpenTelemetry service name set to `ddn-engine` to avoid confusion with
   `graphql-engine`.
 
+- Builds can no longer contain two commands with the same root field name.
+  Previously, one of the two commands would be chosen arbitrarily as the exposed
+  root field. Now, this raises a build-time error.
+
 ## [v2024.09.23]
 
 ### Fixed

v3/crates/metadata-resolve/src/stages/commands/command.rs

@@ -1,10 +1,11 @@
 use crate::helpers::argument::get_argument_kind;
-use crate::helpers::types::{get_type_representation, mk_name};
+use crate::helpers::types::{get_type_representation, mk_name, store_new_graphql_type};
 use crate::stages::{
     boolean_expressions, object_boolean_expressions, scalar_types, type_permissions,
 };
 use crate::types::subgraph::{mk_qualified_type_reference, ArgumentInfo, Qualified};
 use indexmap::IndexMap;
+use lang_graphql::ast::common as ast;
 use open_dds::identifier::SubgraphName;
 
 use super::types::{Command, CommandGraphQlApi};
@@ -13,12 +14,13 @@ use open_dds::commands::CommandV1;
 use open_dds::types::{BaseType, CustomTypeName, TypeName, TypeReference};
 
 use super::error::CommandsError;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
 pub fn resolve_command(
     command: &CommandV1,
     subgraph: &SubgraphName,
     object_types: &BTreeMap<Qualified<CustomTypeName>, type_permissions::ObjectTypeWithPermissions>,
+    graphql_types: &mut BTreeSet<ast::TypeName>,
     scalar_types: &BTreeMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
     object_boolean_expression_types: &BTreeMap<
         Qualified<CustomTypeName>,
@@ -75,14 +77,17 @@ pub fn resolve_command(
     }
 
     let graphql_api = match &command.graphql {
-        None => Ok(None),
-        Some(graphql_definition) => mk_name(graphql_definition.root_field_name.as_ref()).map(|f| {
-            Some(CommandGraphQlApi {
+        Some(graphql_definition) => {
+            let root_field_name = mk_name(graphql_definition.root_field_name.as_ref())?;
+            store_new_graphql_type(graphql_types, Some(&ast::TypeName(root_field_name.clone())))?;
+
+            Ok(Some(CommandGraphQlApi {
                 root_field_kind: graphql_definition.root_field_kind.clone(),
-                root_field_name: f,
+                root_field_name,
                 deprecated: graphql_definition.deprecated.clone(),
-            })
-        }),
+            }))
+        }
+        None => Ok::<Option<CommandGraphQlApi>, CommandsError>(None),
     }?;
 
     Ok(Command {

v3/crates/metadata-resolve/src/stages/commands/mod.rs

@@ -1,3 +1,5 @@
+use lang_graphql::ast::common as ast;
+use std::collections::BTreeSet;
 use std::sync::Arc;
 
 mod command;
@@ -25,6 +27,7 @@ pub fn resolve(
     metadata_accessor: &open_dds::accessor::MetadataAccessor,
     data_connectors: &data_connectors::DataConnectors,
     object_types: &type_permissions::ObjectTypesWithPermissions,
+    graphql_types: &mut BTreeSet<ast::TypeName>,
     scalar_types: &BTreeMap<Qualified<CustomTypeName>, scalar_types::ScalarTypeRepresentation>,
     object_boolean_expression_types: &BTreeMap<
         Qualified<CustomTypeName>,
@@ -44,6 +47,7 @@ pub fn resolve(
             command,
             subgraph,
             object_types,
+            graphql_types,
             scalar_types,
             object_boolean_expression_types,
             boolean_expression_types,

v3/crates/metadata-resolve/src/stages/mod.rs

@@ -164,7 +164,7 @@ pub fn resolve(
         global_id_enabled_types,
         apollo_federation_entity_enabled_types,
         order_by_expressions,
-        graphql_types,
+        mut graphql_types,
         issues,
     } = models::resolve(
         &metadata_accessor,
@@ -187,6 +187,7 @@ pub fn resolve(
         &metadata_accessor,
         &data_connectors,
         &object_types_with_permissions,
+        &mut graphql_types,
         &scalar_types,
         &object_boolean_expression_types,
         &boolean_expression_types,

v3/crates/metadata-resolve/tests/failing/commands/procedures/duplicate_command_name/metadata.json

@@ -0,0 +1,36 @@
+{
+  "version": "v2",
+  "subgraphs": [
+    {
+      "name": "default",
+      "objects": [
+        {
+          "kind": "Command",
+          "version": "v1",
+          "definition": {
+            "name": "hello",
+            "arguments": [],
+            "outputType": "String",
+            "graphql": {
+              "rootFieldName": "hello",
+              "rootFieldKind": "Query"
+            }
+          }
+        },
+        {
+          "kind": "Command",
+          "version": "v1",
+          "definition": {
+            "name": "not_hello",
+            "arguments": [],
+            "outputType": "String",
+            "graphql": {
+              "rootFieldName": "hello",
+              "rootFieldKind": "Query"
+            }
+          }
+        }
+      ]
+    }
+  ]
+}

v3/crates/metadata-resolve/tests/failing/commands/procedures/duplicate_command_name/resolve_error.snap

@@ -0,0 +1,6 @@
+---
+source: crates/metadata-resolve/tests/metadata_golden_tests.rs
+expression: msg
+input_file: crates/metadata-resolve/tests/failing/commands/procedures/duplicate_command_name/metadata.json
+---
+multiple graphql types found with the same name: hello