feat: pretty print results from external-prs (#1529)

* Add a summary message to ossd validation
* Separate messages by address/name
* Provide a way to share informational messages, not just errors
* Fix the status title for the GitHub app

ops/external-prs/package.json

@@ -27,6 +27,7 @@
   },
   "keywords": [],
   "devDependencies": {
+    "@types/lodash": "^4.17.4",
     "@types/node": "^20.11.17",
     "dotenv": "^16.4.1",
     "ts-node": "^10.9.1",
@@ -45,6 +46,7 @@
     "duckdb": "^0.10.1",
     "envfile": "^7.1.0",
     "libsodium-wrappers": "^0.7.13",
+    "lodash": "^4.17.21",
     "mustache": "^4.2.0",
     "octokit": "^3.1.0",
     "oss-directory": "^0.0.12",

ops/external-prs/src/ossd/index.ts

@@ -6,6 +6,7 @@ import { logger } from "../utils/logger.js";
 import { BaseArgs, CommmentCommandHandler } from "../base.js";
 import { loadData, Project, Collection } from "oss-directory";
 import duckdb from "duckdb";
+import _ from "lodash";
 import * as util from "util";
 import * as fs from "fs";
 import * as fsPromise from "fs/promises";
@@ -590,8 +591,38 @@ class OSSDirectoryPullRequest {
     });
     await this.loadValidators(urls);
 
-    const validationErrors: { address: string; error: string }[] = [];
+    // Embedded data structure for storing validation results
+    type ValidationItem = {
+      name: string;
+      messages: string[];
+      errors: string[];
+    };
+    const results: Record<string, ValidationItem> = {};
+    // Add a name to the results
+    const ensureNameInResult = (name: string) => {
+      const item = results[name];
+      if (!item) {
+        results[name] = {
+          name,
+          messages: [],
+          errors: [],
+        };
+      }
+    };
+    // Add an informational message
+    /**
+    const addMessageToResult = (name: string, message: string) => {
+      ensureNameInResult(name);
+      results[name].messages.push(message);
+    };
+    */
+    // Add an error
+    const addErrorToResult = (name: string, message: string) => {
+      ensureNameInResult(name);
+      results[name].errors.push(message);
+    };
 
+    // Run on-chain validations
     for (const item of this.changes.artifacts.toValidate.blockchain) {
       const address = item.address;
       for (const network of item.networks) {
@@ -611,70 +642,58 @@ class OSSDirectoryPullRequest {
         });
         if (item.tags.indexOf("eoa") !== -1) {
           if (!(await validator.isEOA(address))) {
-            validationErrors.push({
-              address: address,
-              error: "is not an EOA",
-            });
+            addErrorToResult(address, "is not an EOA");
           }
         }
         if (item.tags.indexOf("contract") !== -1) {
           if (!(await validator.isContract(address))) {
-            validationErrors.push({
-              address: address,
-              error: "is not a Contract",
-            });
+            addErrorToResult(address, "is not a Contract");
           }
         }
         if (item.tags.indexOf("deployer") !== -1) {
           if (!(await validator.isDeployer(address))) {
-            validationErrors.push({
-              address: address,
-              error: "is not a Deployer",
-            });
+            addErrorToResult(address, "is not a Deployer");
           }
         }
       }
     }
 
-    if (validationErrors.length !== 0) {
-      logger.info({
-        message: "found validation errors",
-        count: validationErrors.length,
-      });
+    // Summarize results
+    const items: ValidationItem[] = _.values(results);
+    const numErrors = _.sumBy(
+      items,
+      (item: ValidationItem) => item.errors.length,
+    );
+    const summaryMessage =
+      numErrors > 0
+        ? `⛔ Found ${numErrors} errors ⛔`
+        : items.length > 0
+          ? "⚠️ Please review validation items before approving ⚠️"
+          : "✅ Good to go as long as status checks pass";
+    const commentBody = await renderMustacheFromFile(
+      relativeDir("messages", "validation-message.md"),
+      {
+        sha: args.sha,
+        summaryMessage,
+        validationItems: items,
+      },
+    );
 
-      await args.appUtils.setStatusComment(
-        args.pr,
-        await renderMustacheFromFile(
-          relativeDir("messages", "validation-errors.md"),
-          {
-            validationErrors: validationErrors,
-            sha: args.sha,
-          },
-        ),
-      );
-
-      await args.appUtils.setCheckStatus({
-        conclusion: CheckConclusion.Failure,
-        name: "validate",
-        head_sha: args.sha,
-        status: CheckStatus.Completed,
-        output: {
-          title: "PR Validation",
-          summary: `Failed to validate with ${validationErrors.length} errors`,
-        },
-      });
-    } else {
-      await args.appUtils.setCheckStatus({
-        conclusion: CheckConclusion.Success,
-        name: "validate",
-        head_sha: args.sha,
-        status: CheckStatus.Completed,
-        output: {
-          title: "PR Validation",
-          summary: "Successfully validated",
-        },
-      });
-    }
+    // Update the PR comment
+    await args.appUtils.setStatusComment(args.pr, commentBody);
+    // Update the PR status
+    await args.appUtils.setCheckStatus({
+      conclusion:
+        numErrors > 0 ? CheckConclusion.Failure : CheckConclusion.Success,
+      name: "validate",
+      head_sha: args.sha,
+      status: CheckStatus.Completed,
+      output: {
+        title:
+          numErrors > 0 ? summaryMessage : "Successfully validated all items",
+        summary: commentBody,
+      },
+    });
   }
 }
 

ops/external-prs/src/ossd/messages/validation-message.md

@@ -0,0 +1,23 @@
+## Validation Results
+
+{{summaryMessage}}
+
+commit `{{sha}}`
+
+---
+
+{{#validationItems}}
+
+### {{name}}
+
+{{#errors}}
+
+- ❌ {{.}}
+  {{/errors}}
+
+{{#messages}}
+
+- 👉 {{.}}
+  {{/messages}}
+
+{{/validationItems}}