Add report

Author: lucy66hw

.github/workflows/convert-proto.yml

@@ -7,6 +7,9 @@ on:
         description: 'OpenSearch version. Leave empty to fetch latest.'
         required: false
         type: string
+  repository_dispatch:
+    types: [spec-updated]
+
 jobs:
   auto-proto-convert:
     runs-on: ubuntu-latest
@@ -29,7 +32,7 @@ jobs:
       - name: Download Release Assets
         uses: robinraju/release-downloader@v1
         with:
-         repository: 'opensearch-project/opensearch-api-specification'
+         repository: 'lucy66hw/opensearch-api-specification'
          latest: true
          fileName: 'opensearch-openapi.yaml'
          tag: 'main-latest'
@@ -119,6 +122,20 @@ jobs:
       - name: Post Process Protobuf
         run: npm run postprocessing
 
+      - name: Generate Merge Report
+        id: merge_report
+        run: |
+          npm run backward-compat -- --dry-run --report /tmp/merge-report.md || true
+          if [ -f /tmp/merge-report.md ]; then
+            # Escape for GitHub Actions multiline output
+            REPORT=$(cat /tmp/merge-report.md)
+            echo "report<<EOF" >> $GITHUB_OUTPUT
+            echo "$REPORT" >> $GITHUB_OUTPUT
+            echo "EOF" >> $GITHUB_OUTPUT
+          else
+            echo "report=No changes detected." >> $GITHUB_OUTPUT
+          fi
+
       - name: Configure Git User
         run: |
           git config --global user.name "github-actions[bot]"
@@ -152,3 +169,7 @@ jobs:
 
             **OpenSearch Version**: ${{ steps.get_opensearch_version.outputs.version }}
             **API Spec Commit**: ${{ steps.get_commit.outputs.latest_commit }}
+
+            ---
+
+            ${{ steps.merge_report.outputs.report }}

tools/proto-convert/src/postprocessing/BackwardCompatibleWriter.ts

@@ -1,22 +1,19 @@
-import { existsSync } from 'fs';
+import { existsSync, writeFileSync } from 'fs';
 import { Command, Option } from '@commander-js/extra-typings';
-import {
-    ProtoMessage,
-    ProtoEnum,
-    BackwardCompatibilityError
-} from './types';
+import { ProtoMessage, ProtoEnum, BackwardCompatibilityError } from './types';
 import { parseProtoFile } from './parser';
 import { mergeMessage, mergeEnum } from './CompatibilityMerger';
 import { writeProtoFile, CUSTOM_MESSAGE_NAMES, CUSTOM_ENUM_NAMES } from './writer';
+import { MergeReporter } from './MergeReporter';
 import logger from '../utils/logger';
 
 export class BackwardCompatibleWriter {
     private existingMessages: ProtoMessage[];
     private existingEnums: ProtoEnum[];
     private incomingMessageMap: Map<string, ProtoMessage> = new Map();
     private incomingEnumMap: Map<string, ProtoEnum> = new Map();
-    private errors: string[] = [];
     private outputPath: string;
+    private reporter: MergeReporter = new MergeReporter();
 
     constructor(existingPath: string, incomingPaths: string[], outputPath: string) {
         this.outputPath = outputPath;
@@ -46,7 +43,7 @@ export class BackwardCompatibleWriter {
         }
     }
 
-    process(): void {
+    process(dryRun: boolean = false): void {
         const finalMessages: ProtoMessage[] = [];
         const finalEnums: ProtoEnum[] = [];
 
@@ -59,7 +56,7 @@ export class BackwardCompatibleWriter {
 
             const incomingMsg = this.incomingMessageMap.get(existingMsg.name);
             if (incomingMsg) {
-                finalMessages.push(mergeMessage(existingMsg, incomingMsg, this.errors));
+                finalMessages.push(mergeMessage(existingMsg, incomingMsg, this.reporter));
                 this.incomingMessageMap.delete(existingMsg.name);
             } else {
                 finalMessages.push(existingMsg);
@@ -75,7 +72,7 @@ export class BackwardCompatibleWriter {
 
             const incomingEnum = this.incomingEnumMap.get(existingEnum.name);
             if (incomingEnum) {
-                finalEnums.push(mergeEnum(existingEnum, incomingEnum));
+                finalEnums.push(mergeEnum(existingEnum, incomingEnum, this.reporter));
                 this.incomingEnumMap.delete(existingEnum.name);
             } else {
                 finalEnums.push(existingEnum);
@@ -96,20 +93,39 @@ export class BackwardCompatibleWriter {
             }
         }
 
-        // Check for errors before writing
-        if (this.errors.length > 0) {
-            logger.error('Backward compatibility errors:');
-            for (const error of this.errors) {
-                logger.error(`  ${error}`);
+        // Check for backward incompatible changes
+        if (this.reporter.hasIncompatibleChanges()) {
+            const errors = this.reporter.getIncompatibleChanges();
+            logger.error('Backward incompatible changes detected:');
+            for (const err of errors) {
+                logger.error(`  ${err.messageName}.${err.fieldName}: ${err.existingType} → ${err.incomingType}`);
             }
-            throw new BackwardCompatibilityError(
-                `Found ${this.errors.length} backward compatibility violation(s).`
-            );
+
+            if (!dryRun) {
+                // In real run mode, throw error and don't write
+                throw new BackwardCompatibilityError(
+                    `Found ${errors.length} backward incompatible change(s). Proto file not updated.`
+                );
+            }
+            // In dry-run mode, continue (report will show the errors)
+            logger.info(`Dry run: ${this.outputPath} would NOT be updated due to incompatible changes`);
+            return;
         }
 
-        // Write output using shared function
-        writeProtoFile(finalMessages, finalEnums, this.outputPath);
-        logger.info(`Updated: ${this.outputPath}`);
+        // Write output using shared function (skip if dry-run)
+        if (dryRun) {
+            logger.info(`Dry run: would update ${this.outputPath}`);
+        } else {
+            writeProtoFile(finalMessages, finalEnums, this.outputPath);
+            logger.info(`Updated: ${this.outputPath}`);
+        }
+    }
+
+    /**
+     * Get the merge reporter for accessing change reports.
+     */
+    getReporter(): MergeReporter {
+        return this.reporter;
     }
 }
 
@@ -124,13 +140,17 @@ if (require.main === module) {
             .argParser((val: string) => val.split(',').map(s => s.trim()))
             .default(['protos/generated/models/aggregated_models.proto', 'protos/generated/services/default_service.proto']))
         .addOption(new Option('-o, --output <path>', 'output proto file').default('protos/schemas/common.proto'))
+        .addOption(new Option('-r, --report <path>', 'output merge report (markdown)'))
+        .addOption(new Option('-d, --dry-run', 'preview changes without writing output file').default(false))
         .allowExcessArguments(false)
         .parse();
 
     type BackwardCompatOpts = {
         existing: string;
         incoming: string[];
         output: string;
+        report?: string;
+        dryRun: boolean;
     };
 
     const opts = command.opts() as BackwardCompatOpts;
@@ -146,17 +166,30 @@ if (require.main === module) {
         process.exit(1);
     }
 
+    const writer = new BackwardCompatibleWriter(
+        opts.existing,
+        opts.incoming,
+        opts.output
+    );
+
     try {
-        const writer = new BackwardCompatibleWriter(
-            opts.existing,
-            opts.incoming,
-            opts.output
-        );
-        writer.process();
+        writer.process(opts.dryRun);
     } catch (error) {
+        // Write report even on error
+        if (opts.report) {
+            writeFileSync(opts.report, writer.getReporter().toMarkdown());
+            logger.info(`Report written: ${opts.report}`);
+        }
+
         if (error instanceof BackwardCompatibilityError) {
             process.exit(1);
         }
         throw error;
     }
+
+    // Write report on success
+    if (opts.report) {
+        writeFileSync(opts.report, writer.getReporter().toMarkdown());
+        logger.info(`Report written: ${opts.report}`);
+    }
 }

tools/proto-convert/src/postprocessing/CompatibilityMerger.ts

@@ -10,6 +10,7 @@ import {
     ProtoOneof,
     Annotation
 } from './types';
+import { MergeReporter, formatField } from './MergeReporter';
 
 const DEPRECATED: Annotation = { name: 'deprecated', value: 'true' };
 
@@ -55,20 +56,25 @@ function addDeprecated<T extends HasAnnotations>(item: T): T {
 }
 
 /**
- * Check if optional added or removed. If so, push error and return true
+ * Check if optional modifier changed and report it.
  */
-function hasOptionalError(
+function checkOptionalChange(
     source: ProtoField,
     upcoming: ProtoField,
     msgName: string,
-    errors: string[]
+    reporter?: MergeReporter
 ): boolean {
     const sourceOptional = source.modifier === 'optional';
     const upcomingOptional = upcoming.modifier === 'optional';
 
     if (sourceOptional !== upcomingOptional) {
-        const change = sourceOptional ? 'removed' : 'added';
-        errors.push(`${msgName}.${source.name}: optional ${change}`);
+        reporter?.addFieldChange({
+            messageName: msgName,
+            changeType: 'optional_error',
+            fieldName: source.name,
+            existingType: formatField(source),
+            incomingType: formatField(upcoming)
+        });
         return true;
     }
     return false;
@@ -88,7 +94,7 @@ function mergeField(
     sourceField: ProtoField,
     upcomingMap: Map<string, ProtoField>,
     msgName: string,
-    errors: string[]
+    reporter?: MergeReporter
 ): ProtoField {
     if (isDeprecated(sourceField)) {
         return sourceField;
@@ -100,30 +106,48 @@ function mergeField(
     if (upcomingField) {
         upcomingMap.delete(baseName);
 
-        if (hasOptionalError(sourceField, upcomingField, msgName, errors)) {
-            return sourceField;
-        }
+        // Report optional modifier changes (but don't stop)
+        checkOptionalChange(sourceField, upcomingField, msgName, reporter);
 
         if (fieldsMatch(sourceField, upcomingField)) {
             return sourceField;
         } else {
             // Type or repeated change - deprecate and version
             const newName = `${baseName}_${getFieldVersion(sourceField.name) + 1}`;
             upcomingMap.set(newName, { ...upcomingField, name: newName });
+            reporter?.addFieldChange({
+                messageName: msgName,
+                changeType: 'type_changed',
+                fieldName: sourceField.name,
+                existingType: formatField(sourceField),
+                incomingType: formatField(upcomingField),
+                versionedName: newName
+            });
             return addDeprecated(sourceField);
         }
     } else {
+        reporter?.addFieldChange({
+            messageName: msgName,
+            changeType: 'removed',
+            fieldName: sourceField.name,
+            existingType: formatField(sourceField)
+        });
         return addDeprecated(sourceField);
     }
 }
 
+/** Check if field name is a versioned name (ends with _N where N is a number) */
+function isVersionedName(name: string): boolean {
+    return /_\d+$/.test(name);
+}
+
 /**
  * Merge a source message with an upcoming message.
  */
 export function mergeMessage(
     sourceMsg: ProtoMessage,
     upcomingMsg: ProtoMessage,
-    errors: string[]
+    reporter?: MergeReporter
 ): ProtoMessage {
     const upcomingByName = new Map(upcomingMsg.fields.map(f => [f.name, f]));
 
@@ -133,7 +157,7 @@ export function mergeMessage(
     // Process regular fields
     for (const sourceField of sourceMsg.fields) {
         maxFieldNumber = Math.max(maxFieldNumber, sourceField.number);
-        mergedFields.push(mergeField(sourceField, upcomingByName, sourceMsg.name, errors));
+        mergedFields.push(mergeField(sourceField, upcomingByName, sourceMsg.name, reporter));
     }
 
     // Process oneofs
@@ -155,7 +179,7 @@ export function mergeMessage(
             const mergedOneofFields: ProtoField[] = [];
             for (const sourceField of sourceOneof.fields) {
                 maxFieldNumber = Math.max(maxFieldNumber, sourceField.number);
-                mergedOneofFields.push(mergeField(sourceField, upcomingOneofByName, sourceMsg.name, errors));
+                mergedOneofFields.push(mergeField(sourceField, upcomingOneofByName, sourceMsg.name, reporter));
             }
 
             mergedOneofs.push({ ...sourceOneof, fields: mergedOneofFields });
@@ -165,6 +189,14 @@ export function mergeMessage(
 
     // Assign field max number to remaining fields.
     for (const field of upcomingByName.values()) {
+        if (!isVersionedName(field.name)) {
+            reporter?.addFieldChange({
+                messageName: sourceMsg.name,
+                changeType: 'added',
+                fieldName: field.name,
+                incomingType: formatField(field)
+            });
+        }
         mergedFields.push({ ...field, number: ++maxFieldNumber });
     }
 
@@ -174,6 +206,14 @@ export function mergeMessage(
             const remaining = oneofMaps.get(oneof.name);
             if (remaining) {
                 for (const field of remaining.values()) {
+                    if (!isVersionedName(field.name)) {
+                        reporter?.addFieldChange({
+                            messageName: sourceMsg.name,
+                            changeType: 'added',
+                            fieldName: `${oneof.name}.${field.name}`,
+                            incomingType: formatField(field)
+                        });
+                    }
                     oneof.fields.push({ ...field, number: ++maxFieldNumber });
                 }
             }
@@ -192,7 +232,8 @@ export function mergeMessage(
  */
 export function mergeEnum(
     sourceEnum: ProtoEnum,
-    upcomingEnum: ProtoEnum
+    upcomingEnum: ProtoEnum,
+    reporter?: MergeReporter
 ): ProtoEnum {
     const sourceValueMap = new Map(sourceEnum.values.map(v => [v.name, v]));
     const upcomingValueMap = new Map(upcomingEnum.values.map(v => [v.name, v]));
@@ -208,12 +249,22 @@ export function mergeEnum(
         if (upcomingValue) {
             mergedValues.push(sourceValue);
         } else {
+            reporter?.addEnumChange({
+                enumName: sourceEnum.name,
+                changeType: 'removed',
+                valueName: sourceValue.name
+            });
             mergedValues.push(addDeprecated(sourceValue));
         }
     }
 
     for (const [valueName, upcomingValue] of upcomingValueMap) {
         if (!sourceValueMap.has(valueName)) {
+            reporter?.addEnumChange({
+                enumName: sourceEnum.name,
+                changeType: 'added',
+                valueName: valueName
+            });
             mergedValues.push({
                 ...upcomingValue,
                 number: ++maxValueNumber