chore(clover): add smoke test instructions for robots to test gcp

bin/clover/AGENTS.md

@@ -36,3 +36,30 @@ buck2 run //bin/si:si schema upload --from-file [filepath]
 ```
 
 This will upload the spec files to make them available as schemas in System Initiative. Schemas that match components that already exist will upload as Working Copies. Existing components will need to be upgraded to take on any changes.
+
+## Testing and Debugging
+
+### Debugging Failed Actions
+
+When an action fails (Create, Update, Delete, Discover, Import, etc.), **always check the func run logs** to understand the actual error:
+
+1. Use `action-list` to find the failed action and its `funcRunId`
+2. Use `func-run-get` with `logs: true` and optionally `result: true` to see:
+   - The actual error message from the cloud provider
+   - The function execution logs showing what happened
+   - The result/output of the function
+
+Example:
+```
+# List actions to find the funcRunId
+action-list --changeSetId <id>
+
+# Get the func run details with logs
+func-run-get --funcRunId <id> --logs true --result true
+```
+
+**Important:** MCP tool errors (like schema validation errors) may hide the actual cloud provider error. Always dig into the func run logs to find the root cause.
+
+### Tests to Run
+
+Look in each pipeline for a smoke.md file that describes which tests to run for that provider.

bin/clover/src/pipelines/gcp/funcs/actions/create.ts

@@ -93,9 +93,11 @@ ${errorText}`
   // - Compute Engine uses "kind" containing "operation"
   // - GKE/Container API uses "operationType" field
   // - Other APIs (API Keys, etc.) use "name" starting with "operations/"
+  // - Cloud Run v2 uses "name" containing "/operations/" and has "metadata" with resource details
   const isLRO = (responseJson.kind && responseJson.kind.includes("operation")) ||
     responseJson.operationType ||
-    (responseJson.name && responseJson.name.startsWith("operations/"));
+    (responseJson.name && responseJson.name.startsWith("operations/")) ||
+    (responseJson.name && responseJson.name.includes("/operations/") && responseJson.metadata);
   if (isLRO) {
     console.log(`[CREATE] LRO detected, polling for completion...`);
 

bin/clover/src/pipelines/gcp/funcs/actions/refresh.ts

@@ -60,10 +60,14 @@ async function refreshViaList(
   let listUrl = buildUrlWithParams(baseUrl, listApiPath, component, projectId, { forList: true });
 
   // Handle parent as query parameter for some APIs
+  // Only add parent if it's declared as a valid query parameter in the API
   if (!listUrl.includes("parent=") && !listApiPath.path.includes("{parent}") && !listApiPath.path.includes("{+parent}")) {
-    const parentValue = resolveParamValue(component, "parent", projectId, true);
-    if (parentValue) {
-      listUrl += (listUrl.includes("?") ? "&" : "?") + `parent=${encodeURIComponent(parentValue)}`;
+    const validQueryParams = listApiPath.queryParams || [];
+    if (validQueryParams.includes("parent")) {
+      const parentValue = resolveParamValue(component, "parent", projectId, true);
+      if (parentValue) {
+        listUrl += (listUrl.includes("?") ? "&" : "?") + `parent=${encodeURIComponent(parentValue)}`;
+      }
     }
   }
 
@@ -95,7 +99,7 @@ async function refreshViaList(
     for (const resource of items || []) {
       const resourceName = resource.name || resource.id;
       if (resourceName === resourceId || resourceName === domainName ||
-          resourceName?.endsWith(`/${resourceId}`) || resourceId.endsWith(`/${resourceName}`)) {
+        resourceName?.endsWith(`/${resourceId}`) || resourceId.endsWith(`/${resourceName}`)) {
         return { payload: normalizeGcpResourceValues(resource), status: "ok" };
       }
     }

bin/clover/src/pipelines/gcp/funcs/actions/update.ts

@@ -34,6 +34,12 @@ async function main(component: Input): Promise<Output> {
     if (currentResource.fingerprint) {
       updatePayload.fingerprint = currentResource.fingerprint;
     }
+
+    // Some assets requires 'name' in the request body even for updates
+    // Always include it from the current resource if available
+    if (currentResource.name && !updatePayload.name) {
+      updatePayload.name = currentResource.name;
+    }
   }
 
   // Try to get update API path first, fall back to patch
@@ -83,12 +89,23 @@ async function main(component: Input): Promise<Output> {
 
   // Many GCP APIs require or benefit from an updateMask query parameter
   // that specifies which fields are being updated
-  const updateFields = Object.keys(updatePayload).filter(k => k !== 'fingerprint');
+  // Exclude 'fingerprint' (used for concurrency) and 'name' (required in body but immutable)
+  const updateFields = Object.keys(updatePayload).filter(k => k !== 'fingerprint' && k !== 'name');
   if (updateFields.length > 0) {
     const updateMask = updateFields.join(',');
     url += (url.includes('?') ? '&' : '?') + `updateMask=${encodeURIComponent(updateMask)}`;
   }
 
+  // Some GCP APIs (like Pub/Sub) use a wrapper request type (e.g., UpdateTopicRequest)
+  // where the payload needs to be wrapped under a specific field (e.g., "topic")
+  // If requestWrapperField is set, wrap the payload accordingly
+  let requestBody = updatePayload;
+  if (updateApiPath.requestWrapperField) {
+    requestBody = {
+      [updateApiPath.requestWrapperField]: updatePayload,
+    };
+  }
+
   // Make the API request with retry logic
   const response = await siExec.withRetry(async () => {
     const resp = await fetch(url, {
@@ -97,7 +114,7 @@ async function main(component: Input): Promise<Output> {
         "Authorization": `Bearer ${token}`,
         "Content-Type": "application/json",
       },
-      body: JSON.stringify(updatePayload),
+      body: JSON.stringify(requestBody),
     });
 
     if (!resp.ok) {

bin/clover/src/pipelines/gcp/funcs/management/discover.ts

@@ -70,10 +70,14 @@ async function main({ thisComponent }: Input): Promise<Output> {
 
   // Handle parent as query parameter for APIs like Resource Manager Folders
   // that don't use parent in the path but require it as a query parameter
+  // Only add parent if it's declared as a valid query parameter in the API
   if (!listUrl.includes("parent=") && !listApiPath.path.includes("{parent}") && !listApiPath.path.includes("{+parent}")) {
-    const parentValue = resolveParamValue(component, "parent", projectId, true);
-    if (parentValue) {
-      listUrl += (listUrl.includes("?") ? "&" : "?") + `parent=${encodeURIComponent(parentValue)}`;
+    const validQueryParams = listApiPath.queryParams || [];
+    if (validQueryParams.includes("parent")) {
+      const parentValue = resolveParamValue(component, "parent", projectId, true);
+      if (parentValue) {
+        listUrl += (listUrl.includes("?") ? "&" : "?") + `parent=${encodeURIComponent(parentValue)}`;
+      }
     }
   }
 
@@ -148,7 +152,13 @@ async function main({ thisComponent }: Input): Promise<Output> {
         } else {
           // Try to get from discovered resource first, then fall back to domain
           paramValue = _.get(resource, [paramName]) ||
-                       _.get(component.properties, ["domain", paramName]);
+            _.get(component.properties, ["domain", paramName]);
+
+          // GCP often returns full URLs for reference fields - extract just the resource name
+          if (paramValue && typeof paramValue === "string" && paramValue.startsWith("https://")) {
+            const urlParts = paramValue.split("/");
+            paramValue = urlParts[urlParts.length - 1];
+          }
         }
 
         if (paramValue) {
@@ -174,15 +184,18 @@ async function main({ thisComponent }: Input): Promise<Output> {
 
     const fullResource = await resourceResponse.json();
 
+    // Normalize the resource to convert full GCP URLs to simple values
+    const normalizedResource = normalizeGcpResourceValues(fullResource);
+
     const properties = {
       si: {
         resourceId,
       },
       domain: {
         ...component.properties?.domain || {},
-        ...fullResource,
+        ...normalizedResource,
       },
-      resource: fullResource,
+      resource: normalizedResource,
     };
 
     // Apply refinement filter

bin/clover/src/pipelines/gcp/funcs/shared/utils.ts

@@ -157,7 +157,7 @@ function isFullResourcePath(resourceId: string, pathTemplate: string): boolean {
 
 function buildUrlWithParams(
   baseUrl: string,
-  apiPath: { path: string; parameterOrder?: string[] },
+  apiPath: { path: string; parameterOrder?: string[]; queryParams?: string[] },
   component: Input,
   projectId: string | undefined,
   options: { resourceId?: string; forList?: boolean } = {}
@@ -187,6 +187,10 @@ function buildUrlWithParams(
       if (paramValue) {
         // Handle {+param} (reserved expansion - don't encode, allows slashes)
         if (url.includes(`{+${paramName}}`)) {
+          // For {+project}, if paramValue doesn't start with "projects/", prepend it
+          if ((paramName === "project" || paramName === "projectId") && !paramValue.startsWith("projects/")) {
+            paramValue = `projects/${paramValue}`;
+          }
           url = url.replace(`{+${paramName}}`, paramValue);
         } else if (url.includes(`{${paramName}}`)) {
           // Handle {param} (simple expansion - encode)
@@ -197,13 +201,24 @@ function buildUrlWithParams(
         }
       }
     }
+  }
 
-    // Add query parameters if any
-    if (queryParams.length > 0) {
-      url += `?${queryParams.join("&")}`;
+  // Handle additional query parameters (e.g., serviceId for Cloud Run)
+  // These are parameters defined with location: "query" in the API spec
+  if (apiPath.queryParams) {
+    for (const paramName of apiPath.queryParams) {
+      const paramValue = resolveParamValue(component, paramName, projectId, options.forList);
+      if (paramValue) {
+        queryParams.push(`${paramName}=${encodeURIComponent(paramValue)}`);
+      }
     }
   }
 
+  // Add query parameters if any
+  if (queryParams.length > 0) {
+    url += `?${queryParams.join("&")}`;
+  }
+
   // Fail fast if URL still contains unresolved placeholders
   // This catches cases where required parameters (like location in parent) are missing
   const unresolvedMatches = [...url.matchAll(/\{[+]?([^}]+)\}/g)];

bin/clover/src/pipelines/gcp/pipeline-steps/addDefaultProps.ts

@@ -76,10 +76,27 @@ export function addDefaultProps(specs: ExpandedPkgSpec[]): ExpandedPkgSpec[] {
         false,
       );
       insertProp.data.hidden = true;
+
+      // Collect query parameters for the insert method
+      // These are parameters with location: "query" that need to be added to the URL
+      const queryParams: string[] = [];
+      if (gcpSchema.methods.insert.parameters) {
+        for (
+          const [paramName, paramDef] of Object.entries(
+            gcpSchema.methods.insert.parameters,
+          )
+        ) {
+          if (paramDef.location === "query") {
+            queryParams.push(paramName);
+          }
+        }
+      }
+
       insertProp.data.defaultValue = JSON.stringify({
         path: gcpSchema.methods.insert.path,
         httpMethod: gcpSchema.methods.insert.httpMethod,
         parameterOrder: gcpSchema.methods.insert.parameterOrder,
+        ...(queryParams.length > 0 ? { queryParams } : {}),
       });
       extraProp.entries.push(insertProp);
     }
@@ -108,10 +125,25 @@ export function addDefaultProps(specs: ExpandedPkgSpec[]): ExpandedPkgSpec[] {
         false,
       );
       patchProp.data.hidden = true;
+
+      // Detect if the patch request uses a wrapper type (e.g., UpdateTopicRequest)
+      // Wrapper types have a field that contains the resource (e.g., "topic") and an "updateMask" field
+      // If detected, store the wrapper field name so the update action can wrap the payload
+      let requestWrapperField: string | undefined;
+      const patchRequest = gcpSchema.methods.patch.request;
+      if (patchRequest?.properties) {
+        const props = Object.keys(patchRequest.properties);
+        // A wrapper type has exactly 2 properties: the resource wrapper field and updateMask
+        if (props.length === 2 && props.includes("updateMask")) {
+          requestWrapperField = props.find((p) => p !== "updateMask");
+        }
+      }
+
       patchProp.data.defaultValue = JSON.stringify({
         path: gcpSchema.methods.patch.path,
         httpMethod: gcpSchema.methods.patch.httpMethod,
         parameterOrder: gcpSchema.methods.patch.parameterOrder,
+        ...(requestWrapperField ? { requestWrapperField } : {}),
       });
       extraProp.entries.push(patchProp);
     }
@@ -134,6 +166,20 @@ export function addDefaultProps(specs: ExpandedPkgSpec[]): ExpandedPkgSpec[] {
     }
 
     if (gcpSchema.methods.list) {
+      // Extract query parameters from the list method
+      const queryParams: string[] = [];
+      if (gcpSchema.methods.list.parameters) {
+        for (
+          const [paramName, paramDef] of Object.entries(
+            gcpSchema.methods.list.parameters,
+          )
+        ) {
+          if (paramDef.location === "query") {
+            queryParams.push(paramName);
+          }
+        }
+      }
+
       const listProp = createScalarProp(
         "listApiPath",
         "string",
@@ -144,6 +190,7 @@ export function addDefaultProps(specs: ExpandedPkgSpec[]): ExpandedPkgSpec[] {
       listProp.data.defaultValue = JSON.stringify({
         path: gcpSchema.methods.list.path,
         parameterOrder: gcpSchema.methods.list.parameterOrder,
+        queryParams: queryParams, // Store valid query parameters
       });
       extraProp.entries.push(listProp);
     }
@@ -237,6 +284,21 @@ export function addDefaultProps(specs: ExpandedPkgSpec[]): ExpandedPkgSpec[] {
         pathParamsSet.add("location");
       }
 
+      // Add query parameters from insert method to pathParameters
+      // These are URL params that should NOT be included in the request body
+      // (e.g., serviceId for Cloud Run services)
+      if (gcpSchema.methods.insert?.parameters) {
+        for (
+          const [paramName, param] of Object.entries(
+            gcpSchema.methods.insert.parameters,
+          )
+        ) {
+          if (param.location === "query") {
+            pathParamsSet.add(paramName);
+          }
+        }
+      }
+
       const propUsageMap: PropUsageMap = {
         createOnly: [],
         updatable: [],