post Saved Objects CI check feedback as a PR comment

.buildkite/scripts/steps/check_saved_objects.sh

@@ -103,12 +103,24 @@ if is_pr; then
     SERVERLESS_BASELINE_FLAG=(--serverless-baseline "$GITHUB_SERVERLESS_BASELINE_SHA")
   fi
 
+  SO_REPORT_PATH="$(mktemp -t so-check-report.XXXXXX).json"
+  CHECK_EXIT=0
+
   if ! is_auto_commit_disabled; then
-    # The step might update files like removed_types.json and/or SO fixtures
-    node scripts/check_saved_objects --baseline "$MERGE_BASE_REV" "${SERVERLESS_BASELINE_FLAG[@]}" --algorithm both --fix
+    # The step might update files like removed_types.json and/or SO fixtures.
+    # `check_for_changed_files` runs unconditionally so that any files produced by --fix
+    # are auto-committed even when the check also reports non-fixable violations.
+    node scripts/check_saved_objects --baseline "$MERGE_BASE_REV" "${SERVERLESS_BASELINE_FLAG[@]}" --algorithm both --report-path "$SO_REPORT_PATH" --fix || CHECK_EXIT=$?
     check_for_changed_files "node scripts/check_saved_objects" true
   else
-    node scripts/check_saved_objects --baseline "$MERGE_BASE_REV" "${SERVERLESS_BASELINE_FLAG[@]}" --algorithm both
+    node scripts/check_saved_objects --baseline "$MERGE_BASE_REV" "${SERVERLESS_BASELINE_FLAG[@]}" --algorithm both --report-path "$SO_REPORT_PATH" || CHECK_EXIT=$?
+  fi
+
+  echo --- Post Saved Objects PR comment
+  ts-node .buildkite/scripts/steps/checks/notify_saved_objects_changes.ts --report-path "$SO_REPORT_PATH" || echo "Warning: failed to post Saved Objects PR notification"
+
+  if [[ "$CHECK_EXIT" -ne 0 ]]; then
+    exit "$CHECK_EXIT"
   fi
 else
   # We are on the 'on-merge' pipeline, the goal is to test against current serverless release,

.buildkite/scripts/steps/checks/notify_saved_objects_changes.test.ts

@@ -0,0 +1,194 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+jest.mock('#pipeline-utils', () => ({
+  upsertComment: jest.fn(),
+}));
+
+import {
+  buildCommentBody,
+  buildFailureBody,
+  buildSuccessBody,
+  type SavedObjectsCheckFinding,
+  type SavedObjectsCheckReport,
+} from './notify_saved_objects_changes';
+
+const finding = (overrides: Partial<SavedObjectsCheckFinding> = {}): SavedObjectsCheckFinding => ({
+  ruleId: 'existing-type/mutated-existing-model-version',
+  severity: 'error',
+  typeName: 'task',
+  message: "Some modelVersions have been updated for SO type 'task'.",
+  fixHint: 'Existing model versions are immutable.',
+  docsAnchor: '#defining-model-versions',
+  ...overrides,
+});
+
+const report = (overrides: Partial<SavedObjectsCheckReport> = {}): SavedObjectsCheckReport => ({
+  status: 'pass',
+  baseline: 'abc123',
+  newTypes: [],
+  updatedTypes: [],
+  removedTypes: [],
+  findings: [],
+  ...overrides,
+});
+
+describe('buildCommentBody', () => {
+  it('returns null on a clean pass with no SO changes', () => {
+    expect(buildCommentBody(report())).toBeNull();
+  });
+
+  it('returns the success body when the run passed and SO types were touched', () => {
+    const body = buildCommentBody(report({ updatedTypes: ['task'] }));
+    expect(body).not.toBeNull();
+    expect(body).toContain('Saved Objects CI check passed');
+  });
+
+  it('returns the failure body when the run failed', () => {
+    const body = buildCommentBody(report({ status: 'fail', findings: [finding()] }));
+    expect(body).toContain('Saved Objects CI check failed');
+  });
+});
+
+describe('buildSuccessBody', () => {
+  it('lists every change category that has entries', () => {
+    const body = buildSuccessBody(
+      report({
+        newTypes: ['shiny-new-type'],
+        updatedTypes: ['task', 'config'],
+        removedTypes: ['old-type'],
+      })
+    );
+
+    expect(body).toContain('**New types:** `shiny-new-type`');
+    expect(body).toContain('**Updated types:** `task`, `config`');
+    expect(body).toContain('**Removed types:** `old-type`');
+  });
+
+  it('includes the 2-step release reminder when types were updated', () => {
+    const body = buildSuccessBody(report({ updatedTypes: ['task'] }));
+    expect(body).toMatch(/2-step release/);
+    expect(body).toContain('https://www.elastic.co/docs/extend/kibana/saved-objects');
+  });
+
+  it('includes the 2-step release reminder when types were removed', () => {
+    const body = buildSuccessBody(report({ removedTypes: ['old-type'] }));
+    expect(body).toMatch(/2-step release/);
+  });
+
+  it('omits the 2-step release reminder for pure new-type additions', () => {
+    const body = buildSuccessBody(report({ newTypes: ['shiny-new-type'] }));
+    expect(body).not.toMatch(/2-step release/);
+    expect(body).toContain('**New types:** `shiny-new-type`');
+  });
+});
+
+describe('buildFailureBody', () => {
+  it('groups findings by typeName and emits per-rule bullets with a docs link', () => {
+    const body = buildFailureBody(
+      report({
+        status: 'fail',
+        findings: [
+          finding({ typeName: 'task', ruleId: 'existing-type/mutated-existing-model-version' }),
+          finding({
+            typeName: 'task',
+            ruleId: 'existing-type/mappings-changed-without-new-model-version',
+            message: 'mappings changed without a new model version',
+            fixHint: 'add a model version',
+          }),
+          finding({
+            typeName: 'config',
+            ruleId: 'new-type/legacy-migrations',
+            message: "New SO type 'config' cannot define legacy migrations",
+            fixHint: 'remove migrations',
+          }),
+        ],
+      })
+    );
+
+    expect(body).toContain('### `task`');
+    expect(body).toContain('### `config`');
+    expect(body).toContain('**[existing-type/mutated-existing-model-version]**');
+    expect(body).toContain('**[new-type/legacy-migrations]**');
+    expect(body).toContain(
+      '([docs](https://www.elastic.co/docs/extend/kibana/saved-objects#defining-model-versions))'
+    );
+    expect(body).toMatch(/3 issue\(s\) across 2 type\(s\)/);
+  });
+
+  it('files findings without a typeName under a General section', () => {
+    const body = buildFailureBody(
+      report({
+        status: 'fail',
+        findings: [
+          finding({ typeName: undefined, ruleId: 'generic', message: 'something blew up' }),
+        ],
+      })
+    );
+
+    expect(body).toContain('### General');
+    expect(body).toContain('**[generic]** something blew up');
+  });
+
+  it('embeds a reproducible local command using the baseline SHA', () => {
+    const body = buildFailureBody(
+      report({ status: 'fail', baseline: 'deadbeef', findings: [finding()] })
+    );
+
+    expect(body).toContain('node scripts/check_saved_objects --baseline deadbeef');
+  });
+
+  it('falls back to a placeholder when no baseline is available', () => {
+    const body = buildFailureBody(
+      report({ status: 'fail', baseline: undefined, findings: [finding()] })
+    );
+
+    expect(body).toContain('node scripts/check_saved_objects --baseline <merge-base-sha>');
+  });
+
+  it('omits the fix hint when not provided', () => {
+    const body = buildFailureBody(
+      report({
+        status: 'fail',
+        findings: [finding({ fixHint: undefined })],
+      })
+    );
+
+    expect(body).not.toContain('_Fix:_');
+  });
+
+  describe('when the run failed but no findings were collected', () => {
+    const originalBuildUrl = process.env.BUILDKITE_BUILD_URL;
+    afterEach(() => {
+      if (originalBuildUrl === undefined) {
+        delete process.env.BUILDKITE_BUILD_URL;
+      } else {
+        process.env.BUILDKITE_BUILD_URL = originalBuildUrl;
+      }
+    });
+
+    it('renders a fallback body that points to the Buildkite build URL when set', () => {
+      process.env.BUILDKITE_BUILD_URL = 'https://buildkite.com/org/pipeline/builds/123';
+      const body = buildFailureBody(report({ status: 'fail', findings: [] }));
+
+      expect(body).toContain('Saved Objects CI check failed');
+      expect(body).toContain('no structured findings were collected');
+      expect(body).toContain('[Buildkite logs](https://buildkite.com/org/pipeline/builds/123)');
+      expect(body).not.toMatch(/0 issue\(s\) across 0 type\(s\)/);
+    });
+
+    it('falls back to a generic phrasing when BUILDKITE_BUILD_URL is unset', () => {
+      delete process.env.BUILDKITE_BUILD_URL;
+      const body = buildFailureBody(report({ status: 'fail', findings: [] }));
+
+      expect(body).toContain('See the Buildkite logs for details');
+      expect(body).not.toContain('[Buildkite logs](');
+    });
+  });
+});