diff --git a/.agents/rules/ecs-schema-standards.mdc b/.agents/rules/ecs-schema-standards.mdc index 0bdd88664..aaea786f7 100644 --- a/.agents/rules/ecs-schema-standards.mdc +++ b/.agents/rules/ecs-schema-standards.mdc @@ -33,3 +33,8 @@ List of mappings; each needs `relation`: `match`, `equivalent`, `related`, `conf - If `expected_values` is set, `example` must be consistent with allowed values. After edits: run `make` and commit `generated/` and generated `docs/reference/` as required by the repo. + +## Automated PR quality review + +For Cursor / agent workflows aimed at reviewer-style feedback (naming, descriptions, examples, duplicates vs base inventory), follow [ecs-pr-review skill](../skills/ecs-pr-review/SKILL.md) and [quality-rules.md](../skills/ecs-pr-review/quality-rules.md). GitHub Actions runs [.github/workflows/pr-review.yml](../../.github/workflows/pr-review.yml). + diff --git a/.agents/skills/ecs-pr-review/SKILL.md b/.agents/skills/ecs-pr-review/SKILL.md new file mode 100644 index 000000000..2298373d8 --- /dev/null +++ b/.agents/skills/ecs-pr-review/SKILL.md @@ -0,0 +1,75 @@ +--- +name: ecs-pr-review +description: >- + Reviews ECS pull requests for schema quality before human review: naming, + description clarity, type strictness, example/pattern/expected_values + consistency, OTel hints, and duplication/conflict checks against a base schema + inventory. Produces severity-graded (High/Medium/Low) actionable findings. +--- + +# ECS PR quality review + +This skill reviews **schema and related changes** in an ECS pull request. It complements **ecs-pr-triage** (routing): triage classifies *where* a change should go; this skill checks *how well* fields are defined. + +## Inputs + +Use what is available in context, or fetch via `gh`: + +- `gh pr view --repo --json title,body,files,additions,deletions,baseRefName,headRefName` +- `gh pr diff --repo ` +- **Schema inventory** at `schema-inventory.tsv` (or equivalent) when provided — flattened field paths from the **base** branch for collision/duplicate reasoning. + +## Execution steps + +### 1. Scope the review + +- Identify files in the diff; prioritize `schemas/**/*.yml`. +- Note any changes under `generated/`, `docs/reference/ecs-*.md`, `scripts/`, `rfcs/`, or `CHANGELOG.next.md` for process rules (see [quality-rules.md](quality-rules.md) **GEN-** rules). + +### 2. Parse schema-relevant hunks + +- For each `schemas/*.yml` hunk, extract **new or modified field entries** (including nested `fields:`) and the **field set** context (`name`, `root`, `reusable` if present). +- Build the **flattened field path** for each leaf the same way as ECS docs (e.g. `agent.version`, `process.parent.pid`, `@timestamp` for root `base`). + +### 3. Apply quality rules + +Walk [quality-rules.md](quality-rules.md) **by category** (types, stability, naming, descriptions, examples, structure, duplicates, generated files, OTel): + +- Map each issue to a **Rule ID** (e.g. `TYP-H01`, `NM-M02`). +- Assign **High**, **Medium**, or **Low** per the table and §10 of [quality-rules.md](quality-rules.md). +- **Evidence** must cite the diff line or inventory row (field path + file). + +### 4. Duplication and conflicts (inventory) + +With **schema-inventory.tsv**: + +- **Columns (expected):** `flat_name`, `type`, `level`, `fieldset`, `source_file`, `description_snippet` (tab-separated). +- **DUP-H01 / DUP-M01:** compare new/changed paths and semantics against inventory; flag collisions and near-duplicates. +- If inventory is missing, still run **structural** checks from the diff only, and state that collision checks were **limited**. + +### 5. Write the report + +Fill [report-template.md](report-template.md): + +- Start with `# ECS PR Quality Review (automated)` as the title line (H1). +- Immediately after the title block, include **`Overall:`** with **`High: N | Medium: M | Low: L`** (counts inferred from findings). +- Include **`### Summary details`** as 2–4 bullets capturing top themes next (GitHub “summary details” section content). +- Use `
…` **per severity bucket** as in the template. +- End with contributor pointers (CONTRIBUTING, guidelines, schemas README). + +### 6. Behaviour constraints + +- **No merge authority:** do not approve, request GitHub Review states, or block CI. +- **Actionable fixes:** each finding ends with concrete YAML or process steps. +- Prefer **fewer, accurate** findings over noisy low-value **Low** items. + +## Related assets + +- [quality-rules.md](quality-rules.md) — canonical rule IDs and severities. +- [report-template.md](report-template.md) — exact output structure. + +## Repo rules cross-reference + +- [ecs-schema-standards.mdc](../../rules/ecs-schema-standards.mdc) +- [ecs-pr-completeness.mdc](../../rules/ecs-pr-completeness.mdc) +- **Routing / RFC:** still use [ecs-pr-triage](../ecs-pr-triage/SKILL.md) when the question is *Direct PR vs RFC*. diff --git a/.agents/skills/ecs-pr-review/quality-rules.md b/.agents/skills/ecs-pr-review/quality-rules.md new file mode 100644 index 000000000..b7dcca43b --- /dev/null +++ b/.agents/skills/ecs-pr-review/quality-rules.md @@ -0,0 +1,112 @@ +# ECS PR Quality Review — quality rules + +Use these rules when reviewing **schema YAML** under `schemas/**/*.yml` and related changes in a pull request. Each rule has an **ID**, **severity** (**High** / **Medium** / **Low**), and **remediation** guidance. + +**Sources of truth (read before inferring):** [schemas/README.md](../../../schemas/README.md), [ecs-schema-standards.mdc](../../rules/ecs-schema-standards.mdc), [docs/reference/ecs-guidelines.md](../../../docs/reference/ecs-guidelines.md), [docs/reference/ecs-conventions.md](../../../docs/reference/ecs-conventions.md), [CONTRIBUTING.md](../../../CONTRIBUTING.md). + +**Allowed field `type` values** (must match Elasticsearch mapping types used in ECS): +`binary`, `boolean`, `keyword`, `constant_keyword`, `wildcard`, `long`, `integer`, `short`, `byte`, `double`, `float`, `half_float`, `scaled_float`, `unsigned_long`, `date`, `date_nanos`, `alias`, `object`, `flattened`, `nested`, `join`, `long_range`, `double_range`, `date_range`, `ip`, `text`, `match_only_text`, `geo_point`, `geo_shape`, `point`, `shape`. + +**Allowed `level` values:** `core`, `extended` (`custom` may appear in legacy contexts; prefer `core`/`extended` for new work per repo tests and docs). + +--- + +## 1. Types and required keys + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **TYP-H01** | High | Invalid `type` | `type` is missing or not in the allowed list above. | Set a valid ECS/ES type (see [ecs-conventions](../../../docs/reference/ecs-conventions.md) for integer/text defaults). | +| **TYP-H02** | High | Invalid `level` | `level` missing or not `core` / `extended`. | Set `level` per field role in the schema. | +| **TYP-H03** | High | Missing required field attributes | Leaf field missing `name`, `description`, `type`, or `level`. | Add all required keys per [schemas/README.md](../../../schemas/README.md). | +| **TYP-H04** | High | `alias` without `path` | `type: alias` and no `path`. | Add `path` to the canonical target field. | +| **TYP-H05** | High | `scaled_float` without `scaling_factor` | `type: scaled_float` and no `scaling_factor`. | Add `scaling_factor` (see existing fields for examples). | + +--- + +## 2. Stability markers + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **STB-H01** | High | `alpha` and `beta` together | Same field or field set has both `alpha` and `beta`. | Remove one marker or split the change per [schemas/README.md](../../../schemas/README.md). | +| **STB-M01** | Medium | Multiline `alpha` / `beta` text | Newline inside `alpha` or `beta` string. | Use a single line (strict/cleaner expectation). | + +--- + +## 3. Naming + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **NM-M01** | Medium | Name token shape | Leaf segment uses uppercase, spaces, or characters other than `[a-z0-9_.@]` (allow `@` only where existing convention, e.g. `@timestamp`). | Use lowercase, underscores; follow [ecs-guidelines](../../../docs/reference/ecs-guidelines.md). | +| **NM-M02** | Medium | Stuttering | Field name repeats the field-set prefix (e.g. `host.host_ip` → should be `host.ip`) or obvious repetition. | Rename to avoid stutter; document rare exceptions (e.g. `host.hostname`). | +| **NM-M03** | Medium | Dubious abbreviation | Abbreviation not in common ECS exceptions (`ip`, `os`, `geo`, `id`, `url`, `http`, `https`, `dns`, `tls`, `api`, `pid`, `uuid`, `sql`, `csv`, `as` field set name, etc.). | Prefer full words unless strongly conventional. | +| **NM-L01** | Low | Minor naming polish | Wording could align better with sibling fields (`.id` vs `.name` patterns). | Align with nearby fields in the same field set. | + +--- + +## 4. Descriptions and `short` + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **DSC-M01** | Medium | Multi-paragraph `description` without `short` | `description` has two+ paragraphs (blank line) and no `short`. | Add a single-line `short` per [schemas/README.md](../../../schemas/README.md). | +| **DSC-M02** | Medium | `short` length / shape | `short` longer than 120 characters or contains newlines. | Trim to ≤120 chars, single line (strict mode / [ecs-schema-standards](../../rules/ecs-schema-standards.mdc)). | +| **DSC-M03** | Medium | Vague or self-referential description | Description only restates the field name with no semantics, or contradicts `type`/`example`. | Clarify meaning, units, population rules, and boundaries. | +| **DSC-L01** | Low | Long single-paragraph description | Very long description without `short` (optional clarity for UIs). | Add `short` for scanability. | + +--- + +## 5. Examples, `pattern`, `expected_values` + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **EX-M01** | Medium | Unquoted composite `example` | `example` is YAML array or mapping (not a quoted string) where composite should be stringified for generators. | Quote as a string (see [schemas/README.md](../../../schemas/README.md)). | +| **EX-M02** | Medium | `example` vs `pattern` | `pattern` set and example string does not `re.match` the pattern. | Fix example or pattern so they agree ([`cleaner.check_example_value`](../../../scripts/schema/cleaner.py)). | +| **EX-M03** | Medium | `example` vs `expected_values` | `expected_values` list present and example not in list (after array-normalize for `normalize: array`). | Align example with a listed value or adjust `expected_values`. | +| **EX-L01** | Low | Missing `example` on new leaf | New leaf field has no `example` where one would aid implementers. | Add a realistic `example`. | + +--- + +## 6. Structural: `object`, `flattened`, reuse + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **STR-M01** | Medium | `object` / `flattened` without children | `type` is `object` or `flattened`, no child `fields`, PR text does not justify opaque shape per [schemas/README.md](../../../schemas/README.md). | Model explicit children OR document homogeneous opaque maps (e.g. string keys/values). | +| **STR-H01** | High | Breaking reuse assumptions | Diff changes `reusable.expected`/`top_level` in ways that break established mounting or duplicates trees incorrectly. | Re-check [schemas/README.md](../../../schemas/README.md) reuse section and RFC needs. | + +--- + +## 7. Duplication and conflicts vs inventory + +The review agent receives a **`schema-inventory.tsv`** (or similar) built from **`schemas/*.yml` at the PR base**. It lists flattened field names (`agent.name`, `@timestamp`, …) and types/descriptions snippets for comparison. + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **DUP-H01** | High | Canonical name collision | New/changed flattened field path duplicates an inventory path that already refers to another distinct definition (excluding identical line from same merged entry). If PR only moves within same RFC flow, reconcile in schema. | Rename or consolidate; avoid conflicting definitions across field sets/locations. | +| **DUP-M01** | Medium | Semantic overlap | New field’s description/examples overlap an existing inventory field (`host.ip` vs new `host.ipv4`) without differentiation. | Reference existing fields, extend with clearer names, or merge concepts. | +| **DUP-M02** | Medium | Duplicate `allowed_values` / category | New `allowed_values` entry name collides or duplicates semantics of an existing entry. | Merge or rename; consider consumers of categorization docs. | + +--- + +## 8. Generated artifacts and docs + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **GEN-H01** | High | Hand-edited generated reference | Diff touches `generated/**` or generator-only `docs/reference/ecs-*.md` without corresponding `schemas/**` (or documented generator-only reason). | Revert manual edits; run `make` / `make check` per [CONTRIBUTING.md](../../../CONTRIBUTING.md). | +| **GEN-M01** | Medium | Changelog expectation | Schema or `scripts/` change with no `CHANGELOG.next.md` update when one is expected per [ecs-pr-completeness](../../rules/ecs-pr-completeness.mdc). | Add entry with `#NNNN` when required. | + +--- + +## 9. OpenTelemetry mappings + +| ID | Severity | Summary | Detection | Remediation | +|----|----------|---------|-----------|-------------| +| **OTL-M01** | Medium | Likely semconv alignment missing | Field clearly parallels a well-known OTel attribute (naming/type) and has no `otel:` block; optional but recommended in [CONTRIBUTING.md](../../../CONTRIBUTING.md). | Add `otel:` with correct `relation` or `relation: na` with justification if intentionally unmapped. | + +--- + +## 10. Severity guidelines for the agent + +- **High:** Breaks schema contract, will or should fail generator/tests, illegal attributes, dangerous collisions, or edits generated-only outputs incorrectly. +- **Medium:** Violates ECS conventions, strict cleaner rules, unclear docs, likely maintainer pushback. +- **Low:** Polish, optional examples, stylistic consistency, non-blocking suggestions. + +When uncertain between two severities, prefer the **higher** one in the checklist and explain why in one sentence. diff --git a/.agents/skills/ecs-pr-review/report-template.md b/.agents/skills/ecs-pr-review/report-template.md new file mode 100644 index 000000000..204713e05 --- /dev/null +++ b/.agents/skills/ecs-pr-review/report-template.md @@ -0,0 +1,81 @@ +# ECS PR Quality Review — report template + +Copy and fill for every automated review run. Produce **filled GitHub-flavored Markdown** (not placeholders). Wrap each severity bucket in `
` for the PR comment unless the tooling posts sections separately. + +````markdown +# ECS PR Quality Review (automated) + +**PR:** #[N] — [title] + +**Overall:** **High:** [H] | **Medium:** [M] | **Low:** [L] +**Scope:** Focus on changed `schemas/**/*.yml` and schema-related tooling/docs in this PR — other files may be listed only if relevant. + +### Summary details + +Brief bullet list ([2–4] bullets): +- Highest-severity themes (e.g. “invalid type”, “collision with inventory”, “missing short”). +- Whether `make check` / strict generator would likely fail. +- One line on duplication or overlap with existing fields (or “none found”). + +--- + +
+High Severity ([H] findings) + +_Use `### HX — [Rule-ID]: [short title]` per finding (X numbering within this section)._ + +### H1 — [RULE-ID]: [Short title] + +- **Field / path:** `[flat field name or field set]` +- **File:** `[path]` +- **Rule:** One sentence tying to ecs-pr-review quality-rules. +- **Evidence:** Quote or paraphrase the diff / inventory line. +- **Fix:** Concrete steps (edit keys, rename, revert generated file, etc.). + +_(Repeat for each High finding.)_ + +_If none: **No High-severity findings.**_ + +
+ +
+Medium Severity ([M] findings) + +### M1 — [RULE-ID]: [Short title] + +- **Field / path:** `...` +- **File:** `...` +- **Rule:** ... +- **Evidence:** ... +- **Fix:** ... + +_(Repeat.)_ + +_If none: **No Medium-severity findings.**_ + +
+ +
+Low Severity ([L] findings) + +### L1 — [RULE-ID]: [Short title] + +- **Field / path:** `...` +- **File:** `...` +- **Suggestion:** ... + +_(Repeat.)_ + +_If none: **No Low-severity findings.**_ + +
+ +--- + +### Notes for contributors + +- Regenerate ECS artifacts after schema edits: **`make`** (or `make check` before push) per [CONTRIBUTING.md](../../../CONTRIBUTING.md). +- Naming and types: [ecs-guidelines](../../../docs/reference/ecs-guidelines.md), [ecs-conventions](../../../docs/reference/ecs-conventions.md), [schemas/README](../../../schemas/README.md). +- Automated review does **not** replace maintainer review or CI. + +```` diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml new file mode 100644 index 000000000..98219ba87 --- /dev/null +++ b/.github/workflows/pr-review.yml @@ -0,0 +1,439 @@ +name: PR Quality Review + +on: + # pull_request_target runs in the base-repo context so secrets are available for + # fork PRs. We checkout the base revision (skills, rules, schema inventory base); + # the PR diff is read via `gh`. + pull_request_target: + types: [opened, synchronize, ready_for_review] + workflow_dispatch: + inputs: + pr_number: + description: "PR number to review" + required: true + type: string + model: + description: "AI model override (provider/model format). Leave empty for LITELLM_MODEL secret." + required: false + default: "llm-gateway/claude-opus-4-6" + type: string + +concurrency: + group: pr-review-${{ github.event.pull_request.number || inputs.pr_number || github.run_id }} + cancel-in-progress: true + +jobs: + # --------------------------------------------------------------------------- + # Phase A — OpenCode + LiteLLM + base schema inventory; read-only GH access. + # --------------------------------------------------------------------------- + review: + name: "Review PR (quality)" + if: >- + github.event_name == 'workflow_dispatch' + || github.event.pull_request.draft == false + runs-on: ubuntu-latest + timeout-minutes: 60 + permissions: + contents: read + pull-requests: read + outputs: + pr_number: ${{ steps.pr.outputs.number }} + + steps: + - name: Resolve PR number + id: pr + env: + EVENT_NAME: ${{ github.event_name }} + PR_FROM_EVENT: ${{ github.event.pull_request.number }} + INPUT_PR_NUMBER: ${{ inputs.pr_number }} + run: | + if [ "$EVENT_NAME" = "workflow_dispatch" ]; then + echo "number=${INPUT_PR_NUMBER}" >> "$GITHUB_OUTPUT" + else + echo "number=${PR_FROM_EVENT}" >> "$GITHUB_OUTPUT" + fi + + - name: Checkout base revision (skills, rules, schemas for inventory) + uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.base.sha || github.sha }} + fetch-depth: 1 + persist-credentials: false + + - name: Generate base schema inventory + run: | + set -euo pipefail + python3 -m pip install --user -q PyYAML==6.0.3 + INV="${GITHUB_WORKSPACE}/pr-review-runtime/schema-inventory.tsv" + mkdir -p "$(dirname "$INV")" + python3 - <<'PY' + import csv + import re + import pathlib + import yaml + import os + + schemas_dir = pathlib.Path("schemas") + out_path = pathlib.Path(os.environ["GITHUB_WORKSPACE"]) / "pr-review-runtime" / "schema-inventory.tsv" + rows = [] + + def one_line(desc): + if desc is None: + return "" + if not isinstance(desc, str): + desc = str(desc) + desc = desc.replace("\n", " ").replace("\r", " ") + desc = re.sub(r"\s+", " ", desc).strip() + return desc[:220] + + def walk(prefix: str, fieldset: str, src: pathlib.Path, entry: dict): + name = entry.get("name") + if name is None: + return [] + flat = name if prefix == "" else f"{prefix}.{name}" + out = [] + t = entry.get("type") or "" + lvl = entry.get("level") or "" + dsc = entry.get("description") or "" + out.append({ + "flat_name": flat, + "type": t, + "level": lvl, + "fieldset": fieldset, + "source_file": str(src.relative_to(schemas_dir.parent)), + "description_snippet": one_line(dsc), + }) + for sub in entry.get("fields") or []: + out.extend(walk(flat, fieldset, src, sub)) + return out + + for fp in sorted(schemas_dir.glob("*.yml")): + with fp.open(encoding="utf-8") as f: + raw = yaml.safe_load(f.read()) + if not raw: + continue + fs_obj = raw[0] + fs_name = fs_obj.get("name", fp.stem) + root = bool(fs_obj.get("root", False)) + base_prefix = "" if root else fs_name + for fld in fs_obj.get("fields") or []: + rows.extend(walk(base_prefix, fs_name, fp, fld)) + + out_path.parent.mkdir(parents=True, exist_ok=True) + with out_path.open("w", encoding="utf-8", newline="") as fh: + w = csv.writer(fh, delimiter="\t") + w.writerow( + ["flat_name", "type", "level", "fieldset", "source_file", "description_snippet"] + ) + for r in rows: + w.writerow( + [ + r["flat_name"], + r["type"], + r["level"], + r["fieldset"], + r["source_file"], + r["description_snippet"], + ] + ) + print(f"Wrote {len(rows)} inventory rows -> {out_path}") + PY + + - name: Install OpenCode CLI + run: npm install -g opencode-ai@1.4.6 + + - name: Resolve model + id: model + env: + INPUT_MODEL: ${{ inputs.model }} + LITELLM_MODEL: ${{ secrets.LITELLM_MODEL }} + DEFAULT_MODEL: ${{ vars.LITELLM_MODEL_DEFAULT || 'llm-gateway/claude-opus-4-6' }} + run: | + if [ -n "${INPUT_MODEL:-}" ]; then + echo "model=${INPUT_MODEL}" >> "$GITHUB_OUTPUT" + else + echo "model=${LITELLM_MODEL:-${DEFAULT_MODEL}}" >> "$GITHUB_OUTPUT" + fi + + - name: Configure OpenCode for LiteLLM + env: + LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }} + LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }} + LITELLM_MODEL: ${{ steps.model.outputs.model }} + run: | + set -euo pipefail + LITELLM_BASE_URL="${LITELLM_BASE_URL:-https://elastic.litellm-prod.ai}" + if [ -z "${LITELLM_API_KEY:-}" ]; then + echo "::error::LITELLM_API_KEY secret is not set. Add it at Settings → Secrets → Actions." + exit 1 + fi + echo "::add-mask::${LITELLM_API_KEY}" + cat > "$GITHUB_WORKSPACE/opencode.json" < "$DIFF_FILE" + + INV_HEAD_LINES=3500 + INV_DISPLAY="$INV_FILE" + if [ "$(wc -l < "$INV_FILE")" -gt "$INV_HEAD_LINES" ]; then + INV_PARTIAL="$RUNTIME/schema-inventory-partial.tsv" + { + echo "# NOTE: truncated to last ${INV_HEAD_LINES} rows of $(wc -l < "$INV_FILE") total (tab-separated)." + tail -n "$INV_HEAD_LINES" "$INV_FILE" + } > "$INV_PARTIAL" + INV_DISPLAY="$INV_PARTIAL" + fi + + PR_JSON=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json title,files,baseRefName,headRefName 2>/dev/null || echo "{}") + + { + cat < --repo --json title,author,body,files,...` + - `gh pr diff --repo ` + + ## What to do + + Follow the **ecs-pr-review** SKILL: inventory scope, parse schema YAML hunks, apply **quality-rules.md** rules (map every finding to a Rule ID and High/Medium/Low), dedupe overlaps, produce the report exactly per **report-template.md**. + + ## Output (required) + + Write the final **ECS PR Quality Review** as GitHub-flavored markdown to: + + `\`${REPORT_FILE}\`` + + The file MUST: + - Begin with `# ECS PR Quality Review (automated)` (H1 as the title line). + - On the next line or immediately after title, include **Overall:** **`High: N | Medium: M | Low: L`** inferred across findings. + - Include `### Summary details` with **2–4 bullets** capturing top themes before the collapsible severity sections. + - Use `
High Severity (…)` (and similarly Medium, Low), each filled or explicitly stating zero findings. + + If you cannot create the file, output **only** the report markdown to stdout with no preamble. + EOF + + echo "" + echo "---" + echo "" + echo "## Inline asset — ecs-pr-review SKILL.md" + echo "" + cat .agents/skills/ecs-pr-review/SKILL.md + echo "" + echo "### quality-rules.md" + echo "" + cat .agents/skills/ecs-pr-review/quality-rules.md + echo "" + echo "### report-template.md" + echo "" + cat .agents/skills/ecs-pr-review/report-template.md + } > "$PROMPT_FILE" + + echo "=== ECS PR Quality Review (OpenCode) ===" + echo "PR: ${REPO}#${PR_NUMBER}" + echo "Model: ${MODEL}" + echo "" + + if opencode run "$(cat "$PROMPT_FILE")" --model "$MODEL" 2>&1 | tee "$TRANSCRIPT_FILE"; then + echo "[OpenCode completed successfully]" + else + echo "[OpenCode exited with non-zero status — continuing to collect output]" + fi + + if [ ! -s "$REPORT_FILE" ]; then + echo "::warning::Report file missing or empty — using transcript fallback body" + { + echo "# ECS PR Quality Review (automated)" + echo "" + echo "**PR:** #${PR_NUMBER}" + echo "**Note:** Review agent did not write \`pr-review-report.md\`. Raw agent output follows." + echo "" + sed 's/\x1b\[[0-9;]*[a-zA-Z]//g' "$TRANSCRIPT_FILE" + } > "$REPORT_FILE" + fi + + - name: Redact agent output + continue-on-error: true + env: + LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }} + run: | + set -euo pipefail + RUNTIME="$GITHUB_WORKSPACE/pr-review-runtime" + python3 - <<'PY' + import os + import re + from pathlib import Path + + root_path = Path(os.environ["GITHUB_WORKSPACE"]) / "pr-review-runtime" + files = [root_path / "pr-review-report.md", root_path / "agent-transcript.md"] + env_keys = ["LITELLM_API_KEY"] + patterns = [ + (r'(?i)(authorization\s*:\s*bearer)\s+[^\s]{12,}', r'\1 [REDACTED]'), + (r'(?i)(api[_-]?key|token|secret)\s*[:=]\s*[\'"]?[a-z0-9._\-/+=]{12,}[\'"]?', r'\1=[REDACTED]'), + (r'\bghp_[A-Za-z0-9]{20,}\b', '[REDACTED_GITHUB_TOKEN]'), + (r'\bgithub_pat_[A-Za-z0-9_]{20,}\b', '[REDACTED_GITHUB_PAT]'), + (r'\bsk-[A-Za-z0-9]{16,}\b', '[REDACTED_SK_TOKEN]'), + ] + + for path in files: + if not path.is_file(): + continue + text = path.read_text(encoding="utf-8", errors="ignore") + for env_var in env_keys: + key = os.getenv(env_var) + if key: + text = text.replace(key, f"[REDACTED_{env_var}]") + for pattern, replacement in patterns: + text = re.sub(pattern, replacement, text) + path.write_text(text, encoding="utf-8") + PY + + - name: Ensure output directory exists + if: always() + run: | + RUNTIME="$GITHUB_WORKSPACE/pr-review-runtime" + mkdir -p "$RUNTIME" + if [ ! -s "$RUNTIME/pr-review-report.md" ]; then + echo "# ECS PR Quality Review (automated)" > "$RUNTIME/pr-review-report.md" + echo "" >> "$RUNTIME/pr-review-report.md" + echo "Quality review agent did not produce a report for this run." >> "$RUNTIME/pr-review-report.md" + fi + + - name: Upload review output + uses: actions/upload-artifact@v7 + if: always() + with: + name: pr-review-output + path: ${{ github.workspace }}/pr-review-runtime/ + if-no-files-found: warn + retention-days: 7 + + # --------------------------------------------------------------------------- + # Phase B — PR comment only, no AI credentials. + # --------------------------------------------------------------------------- + publish: + name: "Publish review comment" + needs: review + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + pull-requests: write + + steps: + - name: Download review output + uses: actions/download-artifact@v8 + with: + name: pr-review-output + path: /tmp/pr-review-output + + - name: Post quality review comment on PR + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ needs.review.outputs.pr_number }} + REPO: ${{ github.repository }} + run: | + set -euo pipefail + + REPORT_FILE="/tmp/pr-review-output/pr-review-report.md" + RUN_URL="${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + + if [ ! -f "$REPORT_FILE" ] || [ ! -s "$REPORT_FILE" ]; then + gh pr comment "$PR_NUMBER" --repo "$REPO" --body "## ECS PR Quality Review + + Automated review did not produce a report. See [workflow run](${RUN_URL}) for logs. + + --- + *Posted by [PR Quality Review workflow](${RUN_URL})*" + exit 0 + fi + + { + cat "$REPORT_FILE" + echo "" + echo "---" + echo "*Posted by [PR Quality Review workflow](${RUN_URL})*" + } > /tmp/pr-review-comment.md + + gh pr comment "$PR_NUMBER" --repo "$REPO" --body-file /tmp/pr-review-comment.md + echo "::notice::Posted PR quality review comment on #${PR_NUMBER}"