From d984eba115686e2b89ba45db341e46135de9e530 Mon Sep 17 00:00:00 2001 From: Mark Cornmesser Date: Thu, 28 May 2026 15:23:50 -0700 Subject: [PATCH 1/5] Add Herald reporter scaffold (no-op + fixture-commit test workflow) First pass at the RelOps Herald change-event reporter, focused on producing and reviewing the JSON output before any Herald service exists. - .github/workflows/herald-reporter.yml: placeholder; triggers on push:master but no-ops. Will be replaced with the production pipeline once the JSON output is approved. - .github/workflows/herald-real-commits-test.yml: runs the full pipeline against two fixture commits and uploads event.json artifacts. - .github/herald/build_event.py: maps changed paths to Herald entities (excluding r10k_modules and staging/alpha), calls GitHub Models for the AI summary, validates the assembled event against the schema. - .github/herald/event.schema.json: Herald change-event schema v1. The AI summary uses actions/ai-inference against GitHub Models via the workflow's GITHUB_TOKEN, so no Anthropic API key is required. --- .github/herald/build_event.py | 195 ++++++++++++++++++ .github/herald/event.schema.json | 139 +++++++++++++ .../workflows/herald-real-commits-test.yml | 185 +++++++++++++++++ .github/workflows/herald-reporter.yml | 29 +++ 4 files changed, 548 insertions(+) create mode 100644 .github/herald/build_event.py create mode 100644 .github/herald/event.schema.json create mode 100644 .github/workflows/herald-real-commits-test.yml create mode 100644 .github/workflows/herald-reporter.yml diff --git a/.github/herald/build_event.py b/.github/herald/build_event.py new file mode 100644 index 000000000..b67b9f7f1 --- /dev/null +++ b/.github/herald/build_event.py @@ -0,0 +1,195 @@ +#!/usr/bin/env python3 +"""Herald reporter helper: map ronin_puppet file paths to entities and assemble +the change-event JSON that conforms to event.schema.json. + +Two subcommands: + map-entities reads a list of changed paths, writes entities.json + assemble combines entities + commit metadata + AI response into event.json +""" + +import argparse +import json +import re +import sys +from pathlib import Path + +import jsonschema + +ROLE_RE = re.compile(r"^modules/roles_profiles/manifests/roles/([^/]+)\.pp$") +PROFILE_RE = re.compile(r"^modules/roles_profiles/manifests/profiles/([^/]+)\.pp$") +ROLE_HIERA_RE = re.compile(r"^data/roles/([^/]+)\.yaml$") +OS_DATA_RE = re.compile(r"^data/os/([^/]+)\.yaml$") +COMMON_DATA_RE = re.compile(r"^data/(common)\.yaml$") +MODULE_RE = re.compile(r"^modules/(?!roles_profiles/)([^/]+)/") + +# Per proposal: staging/alpha images, pools, and roles are excluded from output. +STAGING_ALPHA_RE = re.compile(r"(_staging$|alpha)", re.IGNORECASE) + + +def map_files_to_entities(paths): + entities = {} + patterns = [ + (ROLE_RE, "role"), + (PROFILE_RE, "profile"), + (ROLE_HIERA_RE, "role-hiera"), + (OS_DATA_RE, "os-data"), + (COMMON_DATA_RE, "common-data"), + (MODULE_RE, "module"), + ] + + for raw in paths: + path = raw.strip() + if not path or path.startswith("r10k_modules/"): + continue + for regex, etype in patterns: + m = regex.match(path) + if not m: + continue + eid = m.group(1) + if etype in ("role", "profile", "role-hiera") and STAGING_ALPHA_RE.search(eid): + break + entities.setdefault((etype, eid), set()).add(path) + break + + return [ + {"type": etype, "id": eid, "files": sorted(files)} + for (etype, eid), files in sorted(entities.items()) + ] + + +def parse_ai_response(text): + """Returns (parsed_dict, error). On success parsed_dict has description/headline/tags.""" + if not text or not text.strip(): + return None, "empty AI response" + + cleaned = text.strip() + if cleaned.startswith("```"): + lines = cleaned.splitlines() + lines = lines[1:] if lines[0].startswith("```") else lines + if lines and lines[-1].startswith("```"): + lines = lines[:-1] + cleaned = "\n".join(lines).strip() + + try: + obj = json.loads(cleaned) + except json.JSONDecodeError as exc: + return None, f"AI response not valid JSON: {exc}" + + if not isinstance(obj, dict): + return None, "AI response was not a JSON object" + + description = obj.get("description") + if not isinstance(description, str) or not description.strip(): + return None, "AI response missing non-empty 'description' string" + + headline = obj.get("headline") + if headline is not None and not isinstance(headline, str): + headline = None + if isinstance(headline, str): + headline = headline.strip()[:120] or None + + raw_tags = obj.get("tags") or [] + if not isinstance(raw_tags, list): + raw_tags = [] + tags = [] + for t in raw_tags: + if isinstance(t, str) and t.strip() and t.strip() not in tags: + tags.append(t.strip()) + + return {"description": description.strip(), "headline": headline, "tags": tags}, None + + +def build_ai_summary(model, generated_at, ai_outcome, response_text): + base = {"model": model, "generated_at": generated_at} + if ai_outcome != "success": + return {**base, "description": None, "headline": None, "tags": [], + "error": f"ai-inference step outcome: {ai_outcome}"} + + parsed, err = parse_ai_response(response_text) + if err: + return {**base, "description": None, "headline": None, "tags": [], "error": err} + + return {**base, "description": parsed["description"], + "headline": parsed["headline"], "tags": parsed["tags"], "error": None} + + +def cmd_map_entities(args): + paths = Path(args.changed_files).read_text().splitlines() + entities = map_files_to_entities(paths) + Path(args.output).write_text(json.dumps(entities, indent=2) + "\n") + print(f"Mapped {len(paths)} paths to {len(entities)} entities -> {args.output}") + + +def cmd_assemble(args): + entities = json.loads(Path(args.entities_file).read_text()) + if not entities: + print("No entities; refusing to assemble event.", file=sys.stderr) + sys.exit(1) + + response_text = "" + if args.ai_response_file and Path(args.ai_response_file).exists(): + response_text = Path(args.ai_response_file).read_text() + + ai_summary = build_ai_summary( + model=args.ai_model, + generated_at=args.ai_generated_at, + ai_outcome=args.ai_outcome, + response_text=response_text, + ) + + pr_number = int(args.pr_number) if args.pr_number.strip() else None + pr_url = f"https://github.com/{args.source_repo}/pull/{pr_number}" if pr_number else None + + event = { + "schema_version": "1", + "source_repo": args.source_repo, + "commit_sha": args.commit_sha, + "commit_url": args.commit_url, + "pr_number": pr_number, + "pr_url": pr_url, + "actor": args.actor, + "timestamp": args.timestamp, + "commit_subject": args.commit_subject, + "ai_summary": ai_summary, + "entities": entities, + } + + schema = json.loads(Path(args.schema_file).read_text()) + jsonschema.validate(event, schema) + + Path(args.output).write_text(json.dumps(event, indent=2) + "\n") + print(f"Wrote validated event -> {args.output}") + + +def main(): + parser = argparse.ArgumentParser() + sub = parser.add_subparsers(dest="cmd", required=True) + + p_map = sub.add_parser("map-entities") + p_map.add_argument("--changed-files", required=True) + p_map.add_argument("--output", required=True) + p_map.set_defaults(func=cmd_map_entities) + + p_asm = sub.add_parser("assemble") + p_asm.add_argument("--schema-file", required=True) + p_asm.add_argument("--source-repo", required=True) + p_asm.add_argument("--commit-sha", required=True) + p_asm.add_argument("--commit-url", required=True) + p_asm.add_argument("--pr-number", default="") + p_asm.add_argument("--actor", required=True) + p_asm.add_argument("--timestamp", required=True) + p_asm.add_argument("--commit-subject", required=True) + p_asm.add_argument("--entities-file", required=True) + p_asm.add_argument("--ai-model", required=True) + p_asm.add_argument("--ai-generated-at", required=True) + p_asm.add_argument("--ai-outcome", required=True) + p_asm.add_argument("--ai-response-file", default="") + p_asm.add_argument("--output", required=True) + p_asm.set_defaults(func=cmd_assemble) + + args = parser.parse_args() + args.func(args) + + +if __name__ == "__main__": + main() diff --git a/.github/herald/event.schema.json b/.github/herald/event.schema.json new file mode 100644 index 000000000..d8ceb1123 --- /dev/null +++ b/.github/herald/event.schema.json @@ -0,0 +1,139 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/mozilla-platform-ops/relops-herald/schema/event.schema.json", + "title": "Herald Change Event", + "description": "Structured change event emitted by a reporter workflow (e.g., ronin_puppet) on merge to its default branch. Consumed by Herald to render Markdown changelogs and (later) Slack digests.", + "type": "object", + "required": [ + "schema_version", + "source_repo", + "commit_sha", + "commit_url", + "actor", + "timestamp", + "commit_subject", + "ai_summary", + "entities" + ], + "additionalProperties": false, + "properties": { + "schema_version": { + "type": "string", + "const": "1", + "description": "Schema version. Bump when breaking changes are made." + }, + "source_repo": { + "type": "string", + "pattern": "^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$", + "description": "GitHub / of the reporter." + }, + "commit_sha": { + "type": "string", + "pattern": "^[a-f0-9]{40}$", + "description": "Full 40-char SHA of the merged commit on the default branch." + }, + "commit_url": { + "type": "string", + "format": "uri", + "description": "Permalink to the commit on GitHub." + }, + "pr_number": { + "type": ["integer", "null"], + "minimum": 1, + "description": "PR number that merged this commit, if discoverable. Null for direct pushes." + }, + "pr_url": { + "type": ["string", "null"], + "format": "uri", + "description": "Permalink to the PR. Null when pr_number is null." + }, + "actor": { + "type": "string", + "minLength": 1, + "description": "GitHub login of the user who authored the merge." + }, + "timestamp": { + "type": "string", + "format": "date-time", + "description": "RFC 3339 timestamp of the merge commit (UTC)." + }, + "commit_subject": { + "type": "string", + "minLength": 1, + "description": "First line of the commit message." + }, + "ai_summary": { + "type": "object", + "required": ["model", "generated_at", "description", "error"], + "additionalProperties": false, + "properties": { + "model": { + "type": "string", + "minLength": 1, + "description": "Model ID used for generation (e.g., 'claude-opus-4-7', 'codex-2026-04'). Recorded for debugging quality regressions." + }, + "generated_at": { + "type": "string", + "format": "date-time", + "description": "RFC 3339 timestamp when the AI call completed (UTC)." + }, + "description": { + "type": ["string", "null"], + "description": "Long-form Markdown summary (1-3 sentences). Null when the AI call failed; in that case 'error' is non-null." + }, + "headline": { + "type": ["string", "null"], + "maxLength": 120, + "description": "Short summary (~80 chars) for Slack digests and central activity rows. Optional." + }, + "tags": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "uniqueItems": true, + "description": "Free-form labels proposed by the AI (e.g., 'dependency-bump', 'security'). Optional; safe to ignore in POC rendering." + }, + "error": { + "type": ["string", "null"], + "description": "Non-null only when the AI call failed. Crier renders a stub entry in that case rather than dropping the change." + } + }, + "allOf": [ + { + "description": "If description is null, error must be non-null (and vice versa).", + "oneOf": [ + { "properties": { "description": { "type": "string" }, "error": { "type": "null" } } }, + { "properties": { "description": { "type": "null" }, "error": { "type": "string", "minLength": 1 } } } + ] + } + ] + }, + "entities": { + "type": "array", + "minItems": 1, + "description": "Entities impacted by this commit, derived deterministically from changed file paths.", + "items": { + "type": "object", + "required": ["type", "id", "files"], + "additionalProperties": false, + "properties": { + "type": { + "type": "string", + "enum": ["role", "profile", "module", "role-hiera", "os-data", "common-data"], + "description": "Entity category. Set is intentionally small for the POC; new types added per reporter as needed." + }, + "id": { + "type": "string", + "minLength": 1, + "description": "Stable identifier for the entity within its type (e.g., role name 'gecko_t_linux_2404_talos')." + }, + "files": { + "type": "array", + "minItems": 1, + "items": { "type": "string", "minLength": 1 }, + "description": "Repo-relative paths of files in this commit that mapped to this entity." + } + } + } + } + } +} diff --git a/.github/workflows/herald-real-commits-test.yml b/.github/workflows/herald-real-commits-test.yml new file mode 100644 index 000000000..429346e8e --- /dev/null +++ b/.github/workflows/herald-real-commits-test.yml @@ -0,0 +1,185 @@ +# Herald real-commits test workflow. +# +# Runs the Herald reporter pipeline against two fixture commits from +# ronin_puppet history. For each commit it computes the diff vs the parent, +# maps changed files to entities, calls GitHub Models to generate the AI +# summary, assembles the event JSON, validates against the schema, and +# uploads it as a workflow artifact. +# +# This is the workflow we use to evaluate what Herald change events will +# look like in practice, before wiring the production reporter on merges +# to master. + +name: herald-real-commits-test + +on: + workflow_dispatch: + pull_request: + branches: [master] + paths: + - '.github/herald/**' + - '.github/workflows/herald-real-commits-test.yml' + +permissions: + contents: read + pull-requests: read + models: read + +jobs: + test-commit: + name: Generate event for ${{ matrix.sha }} + if: github.event.pull_request.draft == false || github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + sha: + - f31351e93e3b6ed9c483bc58c63931843adbcaec + - 757b34d34d03da51d68cef6f9eb888f061dbfeb1 + steps: + - name: Checkout (full history needed for fixture commits) + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + fetch-depth: 0 + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 + with: + python-version: '3.11' + + - name: Install dependencies + run: pip install --quiet jsonschema==4.21.1 + + - name: Verify fixture commit is present + run: git cat-file -e ${{ matrix.sha }}^{commit} + + - name: Collect commit metadata and diff + id: meta + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + SHA: ${{ matrix.sha }} + run: | + set -euo pipefail + git diff --name-only "${SHA}^" "${SHA}" > changed_files.txt + git diff "${SHA}^" "${SHA}" | head -c 50000 > diff.txt + + { + echo "subject<> "$GITHUB_OUTPUT" + + echo "timestamp=$(git log -1 --format=%cI "${SHA}")" >> "$GITHUB_OUTPUT" + + # Prefer the commit's GitHub login; fall back to the git author name. + ACTOR=$(gh api "/repos/${{ github.repository }}/commits/${SHA}" \ + --jq '.author.login // empty' 2>/dev/null || true) + if [ -z "${ACTOR}" ]; then + ACTOR=$(git log -1 --format=%an "${SHA}") + fi + echo "actor=${ACTOR}" >> "$GITHUB_OUTPUT" + + PR_NUMBER=$(gh api "/repos/${{ github.repository }}/commits/${SHA}/pulls" \ + --jq '[.[] | select(.merged_at != null)][0].number // empty' 2>/dev/null || true) + echo "pr_number=${PR_NUMBER:-}" >> "$GITHUB_OUTPUT" + + echo "::group::Changed files" + cat changed_files.txt + echo "::endgroup::" + + - name: Map changed files to Herald entities + id: entities + run: | + set -euo pipefail + python .github/herald/build_event.py map-entities \ + --changed-files changed_files.txt \ + --output entities.json + COUNT=$(python -c "import json; print(len(json.load(open('entities.json'))))") + echo "count=${COUNT}" >> "$GITHUB_OUTPUT" + echo "::group::entities.json" + cat entities.json + echo "::endgroup::" + if [ "${COUNT}" = "0" ]; then + echo "::warning::No Herald-relevant entities for ${{ matrix.sha }}; nothing to summarize." + fi + + - name: Build AI prompt + if: steps.entities.outputs.count != '0' + run: | + set -euo pipefail + { + echo "You are summarizing a Mozilla RelOps ronin_puppet commit for a changelog." + echo "" + echo "Commit subject: ${{ steps.meta.outputs.subject }}" + echo "" + echo "Impacted entities (type/id):" + python -c "import json; [print(f\" - {e['type']}/{e['id']}\") for e in json.load(open('entities.json'))]" + echo "" + echo "Changed files:" + sed 's/^/ - /' changed_files.txt + echo "" + echo "Diff (may be truncated at 50KB):" + echo '```diff' + cat diff.txt + echo '```' + echo "" + echo "Return ONLY a JSON object, no code fences, with exactly these keys:" + echo " - description: 1-3 sentence Markdown summary of what changed and why (if inferable)" + echo " - headline: short summary <= 80 chars suitable for a Slack digest line" + echo " - tags: array of short kebab-case tags (e.g. \"dependency-bump\", \"hiera\", \"security\")" + } > prompt.txt + echo "::group::prompt.txt" + cat prompt.txt + echo "::endgroup::" + + - name: Generate AI summary + id: ai + if: steps.entities.outputs.count != '0' + continue-on-error: true + uses: actions/ai-inference@a7805884c80886efc241e94a5351df715968a0ad # v2.1.1 + with: + model: anthropic/claude-sonnet-4 + prompt-file: prompt.txt + max-completion-tokens: 600 + token: ${{ secrets.GITHUB_TOKEN }} + + - name: Assemble and validate event JSON + if: steps.entities.outputs.count != '0' + run: | + set -euo pipefail + AI_RESPONSE_FILE="${{ steps.ai.outputs.response-file }}" + if [ -z "${AI_RESPONSE_FILE}" ] || [ ! -f "${AI_RESPONSE_FILE}" ]; then + AI_RESPONSE_FILE="" + else + echo "::group::AI response" + cat "${AI_RESPONSE_FILE}" + echo + echo "::endgroup::" + fi + python .github/herald/build_event.py assemble \ + --schema-file .github/herald/event.schema.json \ + --source-repo "${{ github.repository }}" \ + --commit-sha "${{ matrix.sha }}" \ + --commit-url "https://github.com/${{ github.repository }}/commit/${{ matrix.sha }}" \ + --pr-number "${{ steps.meta.outputs.pr_number }}" \ + --actor "${{ steps.meta.outputs.actor }}" \ + --timestamp "${{ steps.meta.outputs.timestamp }}" \ + --commit-subject "${{ steps.meta.outputs.subject }}" \ + --entities-file entities.json \ + --ai-model "anthropic/claude-sonnet-4" \ + --ai-generated-at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ + --ai-outcome "${{ steps.ai.outcome }}" \ + --ai-response-file "${AI_RESPONSE_FILE}" \ + --output event.json + echo "::group::event.json" + cat event.json + echo "::endgroup::" + + - name: Upload event artifact + if: steps.entities.outputs.count != '0' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: herald-event-${{ matrix.sha }} + path: event.json + retention-days: 30 diff --git a/.github/workflows/herald-reporter.yml b/.github/workflows/herald-reporter.yml new file mode 100644 index 000000000..8ebebda77 --- /dev/null +++ b/.github/workflows/herald-reporter.yml @@ -0,0 +1,29 @@ +# Herald reporter (NO-OP STUB). +# +# Triggers on merges to master so the workflow path is reserved, but does +# nothing yet. The real change-event emission is being prototyped in +# herald-real-commits-test.yml; once that produces JSON we are happy with, +# this file will be replaced with the production pipeline. +# +# See .github/herald/build_event.py and .github/herald/event.schema.json. + +name: herald-reporter + +on: + push: + branches: [master] + +permissions: + contents: read + +jobs: + stub: + name: Herald reporter (no-op) + runs-on: ubuntu-latest + steps: + - name: Log stub invocation + run: | + echo "::notice::Herald reporter is scaffolded but in no-op mode." + echo "Commit: ${{ github.sha }}" + echo "Actor: ${{ github.actor }}" + echo "Run the herald-real-commits-test workflow to exercise the real pipeline." From 8b16625c40bb69faec79814a1fe93d6a2c1156f4 Mon Sep 17 00:00:00 2001 From: Mark Cornmesser Date: Thu, 28 May 2026 15:29:13 -0700 Subject: [PATCH 2/5] Switch Herald AI model to openai/gpt-4o-mini GitHub Models catalog for mozilla-platform-ops does not include Anthropic publishers; the initial run failed with '404 Unknown model: anthropic/claude-sonnet-4'. Swapping to openai/gpt-4o-mini, which is present in the catalog and accessible via GITHUB_TOKEN (no API key required). --- .github/workflows/herald-real-commits-test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/herald-real-commits-test.yml b/.github/workflows/herald-real-commits-test.yml index 429346e8e..1caf55fad 100644 --- a/.github/workflows/herald-real-commits-test.yml +++ b/.github/workflows/herald-real-commits-test.yml @@ -139,7 +139,7 @@ jobs: continue-on-error: true uses: actions/ai-inference@a7805884c80886efc241e94a5351df715968a0ad # v2.1.1 with: - model: anthropic/claude-sonnet-4 + model: openai/gpt-4o-mini prompt-file: prompt.txt max-completion-tokens: 600 token: ${{ secrets.GITHUB_TOKEN }} @@ -167,7 +167,7 @@ jobs: --timestamp "${{ steps.meta.outputs.timestamp }}" \ --commit-subject "${{ steps.meta.outputs.subject }}" \ --entities-file entities.json \ - --ai-model "anthropic/claude-sonnet-4" \ + --ai-model "openai/gpt-4o-mini" \ --ai-generated-at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ --ai-outcome "${{ steps.ai.outcome }}" \ --ai-response-file "${AI_RESPONSE_FILE}" \ From 2037a549745386994ec9f1c2d6045bde5547d05b Mon Sep 17 00:00:00 2001 From: Mark Cornmesser Date: Fri, 29 May 2026 15:00:54 -0700 Subject: [PATCH 3/5] Switch Herald AI step to direct Anthropic API (Claude Sonnet 4.6) GitHub Models is not enabled for mozilla-platform-ops, and Anthropic publishers are not in the GitHub Models catalog anyway. Now that an ANTHROPIC_API_KEY org/repo secret has been added, swap the AI step from actions/ai-inference to a plain bash + curl call against api.anthropic.com/v1/messages. Avoids the org Actions allowlist constraint that would apply to anthropics/claude-code-action. Model: claude-sonnet-4-6 (matches the pattern in mozilla/bigquery-etl's claude-review workflow). continue-on-error preserves the graceful failure path: if the curl step fails, the assemble step still produces a schema-valid stub with description=null, error=. --- .../workflows/herald-real-commits-test.yml | 71 +++++++++++++++---- 1 file changed, 59 insertions(+), 12 deletions(-) diff --git a/.github/workflows/herald-real-commits-test.yml b/.github/workflows/herald-real-commits-test.yml index 1caf55fad..32fcd3fc5 100644 --- a/.github/workflows/herald-real-commits-test.yml +++ b/.github/workflows/herald-real-commits-test.yml @@ -133,29 +133,76 @@ jobs: cat prompt.txt echo "::endgroup::" - - name: Generate AI summary + - name: Generate AI summary (Anthropic Messages API) id: ai if: steps.entities.outputs.count != '0' continue-on-error: true - uses: actions/ai-inference@a7805884c80886efc241e94a5351df715968a0ad # v2.1.1 - with: - model: openai/gpt-4o-mini - prompt-file: prompt.txt - max-completion-tokens: 600 - token: ${{ secrets.GITHUB_TOKEN }} + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + MODEL: claude-sonnet-4-6 + run: | + set -euo pipefail + if [ -z "${ANTHROPIC_API_KEY:-}" ]; then + echo "::error::ANTHROPIC_API_KEY is not set; AI step cannot run." + exit 1 + fi + + SYSTEM_PROMPT='You are a concise technical writer. Return only a JSON object — no prose, no Markdown code fences — with keys "description", "headline", "tags".' + + jq -n \ + --arg model "${MODEL}" \ + --argjson max_tokens 600 \ + --arg system "${SYSTEM_PROMPT}" \ + --rawfile user_prompt prompt.txt \ + '{model: $model, max_tokens: $max_tokens, system: $system, messages: [{role: "user", content: $user_prompt}]}' \ + > request.json + + # Call Anthropic; capture both response and HTTP status. Do not --fail + # so we can inspect the body on non-2xx. + HTTP_CODE=$(curl --silent --show-error \ + --output response.json \ + --write-out '%{http_code}' \ + --max-time 60 \ + --header "x-api-key: ${ANTHROPIC_API_KEY}" \ + --header "anthropic-version: 2023-06-01" \ + --header "content-type: application/json" \ + --data @request.json \ + https://api.anthropic.com/v1/messages) + + echo "HTTP status: ${HTTP_CODE}" + echo "::group::response.json" + cat response.json + echo + echo "::endgroup::" + + if [ "${HTTP_CODE}" != "200" ]; then + echo "::error::Anthropic API returned HTTP ${HTTP_CODE}" + exit 1 + fi + + # Concatenate all text blocks (defensive against multi-block responses). + jq -r '[.content[]? | select(.type == "text") | .text] | join("\n")' \ + response.json > ai_response.txt + + if [ ! -s ai_response.txt ]; then + echo "::error::No text content in Anthropic response" + exit 1 + fi - name: Assemble and validate event JSON if: steps.entities.outputs.count != '0' + env: + MODEL: claude-sonnet-4-6 run: | set -euo pipefail - AI_RESPONSE_FILE="${{ steps.ai.outputs.response-file }}" - if [ -z "${AI_RESPONSE_FILE}" ] || [ ! -f "${AI_RESPONSE_FILE}" ]; then - AI_RESPONSE_FILE="" - else + if [ -f ai_response.txt ]; then + AI_RESPONSE_FILE="$(pwd)/ai_response.txt" echo "::group::AI response" cat "${AI_RESPONSE_FILE}" echo echo "::endgroup::" + else + AI_RESPONSE_FILE="" fi python .github/herald/build_event.py assemble \ --schema-file .github/herald/event.schema.json \ @@ -167,7 +214,7 @@ jobs: --timestamp "${{ steps.meta.outputs.timestamp }}" \ --commit-subject "${{ steps.meta.outputs.subject }}" \ --entities-file entities.json \ - --ai-model "openai/gpt-4o-mini" \ + --ai-model "${MODEL}" \ --ai-generated-at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ --ai-outcome "${{ steps.ai.outcome }}" \ --ai-response-file "${AI_RESPONSE_FILE}" \ From 6b51394f81a6d0f31f4c6c3399c2320230120ac7 Mon Sep 17 00:00:00 2001 From: Mark Cornmesser Date: Fri, 29 May 2026 15:16:07 -0700 Subject: [PATCH 4/5] Add impact mapping: affected worker pools and Azure images For each commit, derive the transitive set of Puppet roles affected and bucket them: roles whose name contains 'azure' are recorded as azure_images (Azure custom images Herald tracks separately); all other roles map 1:1 to Taskcluster worker pools and land in worker_pools. Impact is derived by parsing Puppet manifests with regex: - role/role-hiera change -> that role - profile change -> roles including the profile - module change -> profiles using the module -> roles including those profiles - data/os/.yaml -> roles matching OS substring (linux/win/mac/osx/darwin) - data/common.yaml -> all roles Schema gains a required top-level 'impact' object with worker_pools[] and azure_images[] arrays of role names. Staging/alpha roles continue to be excluded throughout. --- .github/herald/build_event.py | 116 ++++++++++++++++++ .github/herald/event.schema.json | 23 +++- .../workflows/herald-real-commits-test.yml | 1 + 3 files changed, 139 insertions(+), 1 deletion(-) diff --git a/.github/herald/build_event.py b/.github/herald/build_event.py index b67b9f7f1..5ed86a270 100644 --- a/.github/herald/build_event.py +++ b/.github/herald/build_event.py @@ -25,6 +25,28 @@ # Per proposal: staging/alpha images, pools, and roles are excluded from output. STAGING_ALPHA_RE = re.compile(r"(_staging$|alpha)", re.IGNORECASE) +# Impact analysis: parse Puppet manifests to derive transitive role impact. +INCLUDE_PROFILE_RE = re.compile( + r"\b(?:include|require)\s+(?:::)?roles_profiles::profiles::([A-Za-z0-9_]+)" +) +PROFILE_CLASS_RE = re.compile( + r"class\s*\{\s*['\"](?:::)?roles_profiles::profiles::([A-Za-z0-9_]+)" +) +# Module reference inside a profile: include/require/class on a non-roles_profiles name. +MODULE_REF_RE = re.compile( + r"\b(?:include|require)\s+(?:::)?([a-z][A-Za-z0-9_]*)(?:::[A-Za-z0-9_:]+)?" +) +MODULE_CLASS_RE = re.compile( + r"class\s*\{\s*['\"](?:::)?([a-z][A-Za-z0-9_]*)(?:::[A-Za-z0-9_:]+)?['\"]" +) + +# OS data file -> substring(s) in role names that indicate that OS family. +OS_TO_ROLE_HINTS = { + "Darwin": ("mac", "osx", "darwin"), + "Debian": ("linux",), + "Windows": ("win",), +} + def map_files_to_entities(paths): entities = {} @@ -57,6 +79,96 @@ def map_files_to_entities(paths): ] +def index_role_manifests(repo_root): + """Return {role_id: set(profile_ids)} parsed from modules/roles_profiles/manifests/roles/*.pp.""" + roles_dir = Path(repo_root) / "modules/roles_profiles/manifests/roles" + out = {} + if not roles_dir.is_dir(): + return out + for pp in roles_dir.glob("*.pp"): + role_id = pp.stem + if STAGING_ALPHA_RE.search(role_id): + continue + text = pp.read_text(errors="replace") + profiles = set(INCLUDE_PROFILE_RE.findall(text)) | set(PROFILE_CLASS_RE.findall(text)) + out[role_id] = profiles + return out + + +def index_profile_manifests(repo_root): + """Return {profile_id: set(module_ids)} parsed from .../profiles/*.pp. + + A module is any include/require/class target that isn't itself a profile. + """ + profiles_dir = Path(repo_root) / "modules/roles_profiles/manifests/profiles" + out = {} + if not profiles_dir.is_dir(): + return out + for pp in profiles_dir.glob("*.pp"): + profile_id = pp.stem + text = pp.read_text(errors="replace") + refs = set(MODULE_REF_RE.findall(text)) | set(MODULE_CLASS_RE.findall(text)) + # Drop language keywords and profile self-references. + refs.discard("roles_profiles") + refs.discard("include") + refs.discard("require") + refs.discard("class") + out[profile_id] = refs + return out + + +def compute_impact(entities, repo_root): + """Derive the set of affected roles from the touched entities. + + Returns {"worker_pools": [...], "azure_images": [...]} of role names (no + staging/alpha; "azure" in name -> azure_images, otherwise worker_pools). + """ + role_to_profiles = index_role_manifests(repo_root) + profile_to_modules = index_profile_manifests(repo_root) + + # Reverse maps for fast lookup. + profile_to_roles = {} + for role, profs in role_to_profiles.items(): + for p in profs: + profile_to_roles.setdefault(p, set()).add(role) + + module_to_profiles = {} + for prof, mods in profile_to_modules.items(): + for m in mods: + module_to_profiles.setdefault(m, set()).add(prof) + + affected = set() + all_roles = set(role_to_profiles) + + for e in entities: + etype, eid = e["type"], e["id"] + if etype in ("role", "role-hiera"): + if eid in all_roles: + affected.add(eid) + elif etype == "profile": + affected.update(profile_to_roles.get(eid, set())) + elif etype == "module": + for prof in module_to_profiles.get(eid, set()): + affected.update(profile_to_roles.get(prof, set())) + elif etype == "os-data": + hints = OS_TO_ROLE_HINTS.get(eid, ()) + if hints: + for role in all_roles: + name = role.lower() + if any(h in name for h in hints): + affected.add(role) + elif etype == "common-data": + affected.update(all_roles) + + worker_pools, azure_images = [], [] + for role in sorted(affected): + if "azure" in role.lower(): + azure_images.append(role) + else: + worker_pools.append(role) + return {"worker_pools": worker_pools, "azure_images": azure_images} + + def parse_ai_response(text): """Returns (parsed_dict, error). On success parsed_dict has description/headline/tags.""" if not text or not text.strip(): @@ -140,6 +252,8 @@ def cmd_assemble(args): pr_number = int(args.pr_number) if args.pr_number.strip() else None pr_url = f"https://github.com/{args.source_repo}/pull/{pr_number}" if pr_number else None + impact = compute_impact(entities, args.repo_root) + event = { "schema_version": "1", "source_repo": args.source_repo, @@ -152,6 +266,7 @@ def cmd_assemble(args): "commit_subject": args.commit_subject, "ai_summary": ai_summary, "entities": entities, + "impact": impact, } schema = json.loads(Path(args.schema_file).read_text()) @@ -184,6 +299,7 @@ def main(): p_asm.add_argument("--ai-generated-at", required=True) p_asm.add_argument("--ai-outcome", required=True) p_asm.add_argument("--ai-response-file", default="") + p_asm.add_argument("--repo-root", default=".", help="Repo root for impact analysis") p_asm.add_argument("--output", required=True) p_asm.set_defaults(func=cmd_assemble) diff --git a/.github/herald/event.schema.json b/.github/herald/event.schema.json index d8ceb1123..8f36119d5 100644 --- a/.github/herald/event.schema.json +++ b/.github/herald/event.schema.json @@ -13,7 +13,8 @@ "timestamp", "commit_subject", "ai_summary", - "entities" + "entities", + "impact" ], "additionalProperties": false, "properties": { @@ -134,6 +135,26 @@ } } } + }, + "impact": { + "type": "object", + "required": ["worker_pools", "azure_images"], + "additionalProperties": false, + "description": "Roles affected transitively by this commit, bucketed by what they represent. Roles whose name contains 'azure' are treated as Azure custom images Herald keeps track of; all other roles map 1:1 to Taskcluster worker pools.", + "properties": { + "worker_pools": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Role names (== worker pool identifiers) impacted by the change, excluding any role whose name contains 'azure'. Sorted." + }, + "azure_images": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Role names containing 'azure', representing Azure custom images tracked separately from worker pools. Sorted." + } + } } } } diff --git a/.github/workflows/herald-real-commits-test.yml b/.github/workflows/herald-real-commits-test.yml index 32fcd3fc5..cda023905 100644 --- a/.github/workflows/herald-real-commits-test.yml +++ b/.github/workflows/herald-real-commits-test.yml @@ -218,6 +218,7 @@ jobs: --ai-generated-at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ --ai-outcome "${{ steps.ai.outcome }}" \ --ai-response-file "${AI_RESPONSE_FILE}" \ + --repo-root . \ --output event.json echo "::group::event.json" cat event.json From f3850d558b479bdd476dd1eaef5f9ba6ab715692 Mon Sep 17 00:00:00 2001 From: Mark Cornmesser Date: Fri, 29 May 2026 16:54:17 -0700 Subject: [PATCH 5/5] Improve Puppet parsing + add CONVENTIONS.md for AI context Parsing improvements in build_event.py: - Strip Puppet comments before regex matching (drops '# include ...' false matches and /* ... */ blocks). - Add profile -> profile transitive closure so a module/profile change reaches every role whose profile chain pulls it in, not just direct includes. Verified by touching profile 'motd' (only included transitively via OS-base profiles): impact fans out from 8 to 41 roles, which matches reality. New .github/herald/CONVENTIONS.md documents ronin_puppet conventions for the reporter: - Repo shape and entity types - worker-pool vs azure-image (and the 'azure' substring rule) - staging/alpha exclusion - Impact derivation rules per entity type - Terminology guidance for AI summaries The herald-real-commits-test workflow now inlines CONVENTIONS.md into the Claude prompt so descriptions use the same terminology a ronin reviewer would (worker pool, Azure custom image, profile, module, Hiera). --- .github/herald/CONVENTIONS.md | 97 +++++++++++++++++++ .github/herald/build_event.py | 85 ++++++++++++---- .../workflows/herald-real-commits-test.yml | 16 ++- 3 files changed, 174 insertions(+), 24 deletions(-) create mode 100644 .github/herald/CONVENTIONS.md diff --git a/.github/herald/CONVENTIONS.md b/.github/herald/CONVENTIONS.md new file mode 100644 index 000000000..e1782949f --- /dev/null +++ b/.github/herald/CONVENTIONS.md @@ -0,0 +1,97 @@ +# ronin_puppet conventions (for Herald reporter and AI summaries) + +This document describes the ronin_puppet conventions that the Herald +reporter workflow relies on to map a commit's changed files to +**entities** (what the commit directly touched) and **impact** (which +worker pools / Azure custom images are affected transitively). + +The reporter workflow inlines this file into the prompt sent to the +LLM that writes the human-readable summary, so the AI can use the same +terminology and assumptions a ronin reviewer would. + +--- + +## Repo shape + +| Path | Meaning | +|---|---| +| `modules/roles_profiles/manifests/roles/.pp` | A **role**. One Puppet class per machine type; 1:1 with a Taskcluster worker pool unless the name contains `azure` (see below). ~63 roles total. | +| `modules/roles_profiles/manifests/profiles/.pp` | A **profile**. OS-independent interface composed by roles. Profiles may `include`/`require` other profiles, and they reference component modules. | +| `modules//manifests/...` (anything except `modules/roles_profiles/`) | A **module**. Reusable Puppet code (e.g., `linux_snmpd`, `generic_worker`, `macos_xcode_tools`). | +| `data/roles/.yaml` | Per-role Hiera data — **role-hiera**. | +| `data/os/.yaml` (`Debian`, `Darwin`, `Windows`) | Per-OS Hiera data — **os-data**. Applies to every role whose OS matches. | +| `data/common.yaml` | Global Hiera data — **common-data**. Applies to every role. | +| `r10k_modules/**` | Vendored external Puppet modules (pinned via `Puppetfile`). **Excluded** from Herald entity tracking; not authored in this repo. | + +## Entity types emitted in `entities[]` + +The reporter records only **directly-touched** entities here: + +| Entity `type` | `id` source | When | +|---|---|---| +| `role` | filename stem | A role's `.pp` changed | +| `profile` | filename stem | A profile's `.pp` changed | +| `module` | top-level dir name under `modules/` | Any file under a non-`roles_profiles` module changed | +| `role-hiera` | filename stem | A `data/roles/.yaml` changed | +| `os-data` | filename stem | `data/os/.yaml` changed | +| `common-data` | constant `common` | `data/common.yaml` changed | + +Anything else (e.g., `.github/`, `README.md`, integration tests outside a +recognizable role path) does not produce an entity and may produce a +commit with zero entities — those commits are skipped, not emitted. + +## Worker pools vs Azure custom images + +Every role with a name **not** containing `azure` (case-insensitive) +maps 1:1 to a Taskcluster **worker pool** with the same identifier. +Roles whose name contains `azure` are **Azure custom images** — +Herald tracks them separately because their "go-live" semantics differ +from a worker pool deployment. + +This split is captured in `impact.worker_pools[]` and +`impact.azure_images[]`. The split is purely a name match; there is no +other source of truth in this repo. + +## Staging/alpha exclusion + +Roles, profiles, and per-role Hiera files whose names match the +following are **excluded** from both `entities[]` and `impact`: +- name ends with `_staging` (e.g., `gecko_1_b_osx_1015_staging`) +- name contains `alpha` substring (e.g., `win116424h2hwalpha`, + `win116424h2hwrefalpha`) + +Pre-prod and experimental machines should not show up in changelogs +intended for operational visibility. + +## Impact derivation + +`impact.worker_pools[]` and `impact.azure_images[]` are the union of +roles affected by each touched entity, bucketed by the azure rule above. + +| Touched entity | Rule | +|---|---| +| `role` / `role-hiera` | The role itself. | +| `profile` | Every role that transitively includes the profile (closure over `include roles_profiles::profiles::

` and `require roles_profiles::profiles::

` in role manifests, plus the same in profile manifests so profile→profile chains are followed). | +| `module` | Every profile that references the module (`include ::*` / `require ::*` / `class { '::*': ... }`), expanded out through the profile closure to the roles that include any profile in that set. | +| `os-data` for `` | Every role whose name contains one of the OS hints: `Debian` → `linux`; `Darwin` → `mac`, `osx`, `darwin`; `Windows` → `win`. | +| `common-data` | Every (non-staging/alpha) role. | + +OS family inference is a heuristic on the role name — accurate for the +current ronin naming convention (`gecko_t_linux_*`, `gecko_t_osx_*`, +`win*`, `mac*`) but may miss future roles that don't encode OS in their +name. When in doubt, treat impact as a best-effort lower bound. + +## Notes for AI summarization + +When you write the `description` and `headline` for an event: +- Use the exact terminology above: "worker pool" (not "pool"), "Azure + custom image" (not just "image" or "AMI"), "role", "profile", "module", + "Hiera". +- If the commit only touches a module, note which profile(s) consume it + and which roles are downstream (the `impact` block contains that + information). +- Distinguish between an OS-wide change (`data/os/Windows.yaml` → + affects every Windows role) and a per-role override + (`data/roles/.yaml`). +- Don't speculate about rollout timing or staging promotion — Herald + reports what changed in source, not what's running where. diff --git a/.github/herald/build_event.py b/.github/herald/build_event.py index 5ed86a270..021ff596e 100644 --- a/.github/herald/build_event.py +++ b/.github/herald/build_event.py @@ -79,8 +79,23 @@ def map_files_to_entities(paths): ] +def strip_puppet_comments(text): + """Remove Puppet comments before regex matching. + + Drops `# ...` to end of line and `/* ... */` blocks. Naive about strings + that contain `#` characters, which is fine here since we only care about + statement-level include/require/class lines. + """ + text = re.sub(r"/\*.*?\*/", "", text, flags=re.DOTALL) + text = re.sub(r"#.*", "", text) + return text + + def index_role_manifests(repo_root): - """Return {role_id: set(profile_ids)} parsed from modules/roles_profiles/manifests/roles/*.pp.""" + """Return {role_id: set(profile_ids)} parsed from .../roles/*.pp. + + Reads only un-commented include/require lines. Excludes staging/alpha roles. + """ roles_dir = Path(repo_root) / "modules/roles_profiles/manifests/roles" out = {} if not roles_dir.is_dir(): @@ -89,57 +104,87 @@ def index_role_manifests(repo_root): role_id = pp.stem if STAGING_ALPHA_RE.search(role_id): continue - text = pp.read_text(errors="replace") + text = strip_puppet_comments(pp.read_text(errors="replace")) profiles = set(INCLUDE_PROFILE_RE.findall(text)) | set(PROFILE_CLASS_RE.findall(text)) out[role_id] = profiles return out def index_profile_manifests(repo_root): - """Return {profile_id: set(module_ids)} parsed from .../profiles/*.pp. + """Return ({profile_id: set(module_ids)}, {profile_id: set(profile_ids)}). - A module is any include/require/class target that isn't itself a profile. + The first map is profile -> referenced module names. The second is + profile -> profiles it transitively includes (so we can build a closure). """ profiles_dir = Path(repo_root) / "modules/roles_profiles/manifests/profiles" - out = {} + modules_by_profile, profiles_by_profile = {}, {} if not profiles_dir.is_dir(): - return out + return modules_by_profile, profiles_by_profile for pp in profiles_dir.glob("*.pp"): profile_id = pp.stem - text = pp.read_text(errors="replace") + text = strip_puppet_comments(pp.read_text(errors="replace")) + + included_profiles = set(INCLUDE_PROFILE_RE.findall(text)) | set(PROFILE_CLASS_RE.findall(text)) + included_profiles.discard(profile_id) + profiles_by_profile[profile_id] = included_profiles + + # Module refs: every top-level name from include/require/class that + # isn't itself a profile or a Puppet keyword. refs = set(MODULE_REF_RE.findall(text)) | set(MODULE_CLASS_RE.findall(text)) - # Drop language keywords and profile self-references. - refs.discard("roles_profiles") - refs.discard("include") - refs.discard("require") - refs.discard("class") - out[profile_id] = refs + for stop in ("roles_profiles", "include", "require", "class"): + refs.discard(stop) + modules_by_profile[profile_id] = refs + + return modules_by_profile, profiles_by_profile + + +def profile_closure(seed_profiles, profile_to_profiles): + """Expand a set of profiles to include every profile they transitively + include (via profile -> profile chains in profile_to_profiles).""" + out = set(seed_profiles) + frontier = set(seed_profiles) + while frontier: + nxt = set() + for p in frontier: + for child in profile_to_profiles.get(p, ()): + if child not in out: + nxt.add(child) + out.add(child) + frontier = nxt return out def compute_impact(entities, repo_root): """Derive the set of affected roles from the touched entities. - Returns {"worker_pools": [...], "azure_images": [...]} of role names (no - staging/alpha; "azure" in name -> azure_images, otherwise worker_pools). + Returns {"worker_pools": [...], "azure_images": [...]} of role names. + 'azure' in role name (case-insensitive) -> azure_images; else worker_pools. + Profile -> profile transitive includes are followed so a change deep in + the closure reaches every role that pulls it in indirectly. """ role_to_profiles = index_role_manifests(repo_root) - profile_to_modules = index_profile_manifests(repo_root) + profile_to_modules, profile_to_profiles = index_profile_manifests(repo_root) + all_roles = set(role_to_profiles) - # Reverse maps for fast lookup. + # For each role, the full set of profiles it transitively pulls in. + role_profile_closure = { + role: profile_closure(profs, profile_to_profiles) + for role, profs in role_to_profiles.items() + } + + # Reverse map: profile -> roles whose closure contains it. profile_to_roles = {} - for role, profs in role_to_profiles.items(): + for role, profs in role_profile_closure.items(): for p in profs: profile_to_roles.setdefault(p, set()).add(role) + # Reverse map: module -> profiles referencing it. module_to_profiles = {} for prof, mods in profile_to_modules.items(): for m in mods: module_to_profiles.setdefault(m, set()).add(prof) affected = set() - all_roles = set(role_to_profiles) - for e in entities: etype, eid = e["type"], e["id"] if etype in ("role", "role-hiera"): diff --git a/.github/workflows/herald-real-commits-test.yml b/.github/workflows/herald-real-commits-test.yml index cda023905..053b2fb05 100644 --- a/.github/workflows/herald-real-commits-test.yml +++ b/.github/workflows/herald-real-commits-test.yml @@ -111,9 +111,15 @@ jobs: { echo "You are summarizing a Mozilla RelOps ronin_puppet commit for a changelog." echo "" - echo "Commit subject: ${{ steps.meta.outputs.subject }}" + echo "## Repo conventions" echo "" - echo "Impacted entities (type/id):" + cat .github/herald/CONVENTIONS.md + echo "" + echo "## This commit" + echo "" + echo "Subject: ${{ steps.meta.outputs.subject }}" + echo "" + echo "Directly-touched entities (type/id):" python -c "import json; [print(f\" - {e['type']}/{e['id']}\") for e in json.load(open('entities.json'))]" echo "" echo "Changed files:" @@ -124,12 +130,14 @@ jobs: cat diff.txt echo '```' echo "" + echo "## Output" + echo "" echo "Return ONLY a JSON object, no code fences, with exactly these keys:" - echo " - description: 1-3 sentence Markdown summary of what changed and why (if inferable)" + echo " - description: 1-3 sentence Markdown summary using the conventions terminology above" echo " - headline: short summary <= 80 chars suitable for a Slack digest line" echo " - tags: array of short kebab-case tags (e.g. \"dependency-bump\", \"hiera\", \"security\")" } > prompt.txt - echo "::group::prompt.txt" + echo "::group::prompt.txt (size $(wc -c < prompt.txt) bytes)" cat prompt.txt echo "::endgroup::"