feat: implement issue #506 — [ADR-0007] Collapse per-role Class-1 stubs into agent-ingress.yml (pilot)

Merged
DO
10d ago
+382-333

Problem

[ADR-0007] Collapse per-role Class-1 stubs into agent-ingress.yml (pilot)

From the issue: Implements ADR-0007 for this repo: collapse the 5 event-driven Class-1 agentic caller stubs into a single .github/workflows/agent-ingress.yml.

Risk

Medium — changes GitHub Actions workflow behavior, which is exercised only post-merge; verify via the affected workflow runs.

Test plan

Tests added/updated: .github/scripts/tests/agent-ingress-workflow.bats,.github/scripts/tests/pr-review-mention-workflow.bats. Verification: bash scripts/dev-lead-lint.sh (shellcheck --severity=warning) ran pre-commit; the bats suite runs in CI.

Rollback

Revert this PR. No non-revertible side effects (no tags, migrations, or external state).

Monitoring

Watch the affected workflow run(s) in the Actions tab and this PR's Lint check for regressions.

Refs #506 and Refs petry-projects/.github-private#1729

Summary by CodeRabbit

  • Automation
    • Agent-related pull request, review, comment, issue, check, workflow-run, and dispatch events are handled through a unified workflow with event-specific filters.
    • Development leadership, automated reviews, review mentions, and CI failure analysis are routed through that workflow with role-specific concurrency handling.
    • Manual review dispatch supports a pull request URL, dry-run mode, and forced review.
    • Review-mention handling skips bot-sent events, and CI failure analysis skips its own check runs.

§8a pre-merge gate evidence (maintainer-verified 2026-10-09, head 376f04b)

  1. Ruleset re-read at PR time (GET /repos/petry-projects/markets/rules/branches/main): required checks are exactly CodeQL, SonarCloud, agent-shield / AgentShield, dependency-audit / Detect ecosystems. None of the five collapsing roles is required, so no required-check edit is coupled to this merge.
  2. Content vs the ratified package §3 (petry-projects/.github-private#2039). agent-ingress.yml is identical to §3 except TWO deliberate differences: (a) the ci-failure-analyst job's uses: SHA is b5851027… (carrying main's Dependabot bump e390f85) instead of 79747178…; (b) the pr-auto-review job's concurrency.group is pr-auto-review-${{ pull_request.number || check_suite.id || workflow_run.id || 'none' }} (literal cancel-in-progress: true) instead of §3's per-head-SHA slot. (b) is ADR-0010-compliant (literal role prefix, event-payload only, no run_id, literal boolean) and is closer to the deleted stub's behavior: non-PR events get their own slot instead of coalescing per commit, which avoids the cross-PR hazard documented in #1126. Package §3/§4b will be updated to match (tracked in petry-projects/.github-private).
  3. Validators, run for real on this exact file (validate-ingress-if.sh and validate-caller-inputs.sh from .github-private main, with the five reusables fetched at their pinned refs): ingress-if: OK — every agent-ingress job is a thin caller with a pure event-filter if: and bounded concurrency. and caller-inputs: OK — every resolved caller forwards a valid input set. (4 pinned refs checked, 0 unresolved; pr-auto-review, pr-review and ci-failure-analyst report no collision with their reusable's groups). The unmodified ratified §3 YAML passes the same harness, as a control. Run with a jq-flavored yq plus a raw-output shim on bash 5, not in this repo's CI.
  4. All five stubs deleted in this PR: dev-lead, pr-auto-review, pr-review, pr-review-mention, ci-failure-analyst. The six §2b carve-out files are untouched.
  5. pr-review / review shows cancelled: the job-level concurrency (cancel-in-progress: true, same as the deleted stub) superseded an in-flight run. It is a renamed, non-required check.

Known limitations and intentional deviations

This PR is a behavior-preserving collapse of five per-role caller stubs into one agent-ingress.yml (ADR-0007). Every job keeps the original trigger subscription, if: filter, permissions, secrets forwarding and reusable ref. The points below are the places where it is NOT a pure copy, or where a reviewer should know about a pre-existing weakness that the collapse deliberately preserves.

  1. # DEFECTIVE PIN markers (kept on purpose). Two pins were already non-standard before this PR and are carried over unchanged so the collapse stays attributable:
    • pr-review pins @pr-review/stable, a bare channel with no major version (every other first-party pin rides a <name>/v<MAJOR>-<tier> channel). It is exposed to the channel-skew defect class. Follow-up: repoint to the current pr-review/v<MAJOR>-stable channel.
    • ci-failure-analyst pins a bare commit SHA (@b5851027…, commented # main) rather than a moving channel tag, so reusable fixes cannot reach this repo without a hand edit. This PR carries forward main's Dependabot bump of that SHA (from 79747178…). Follow-up: adopt a ci-failure-analyst/v<MAJOR>-stable channel and repoint.
      Changing a pin inside the collapse would make "the collapse is behavior-identical" and "the pin changed" impossible to tell apart on rollback.
  2. pr-review concurrency fallback changed (behavior change). The deleted stub fell back to github.run_id. ADR-0010 forbids run_id in a caller group, so the fallback is now the constant enumerate, with cancel-in-progress: true. Effect: whole-repo review sweeps (a workflow_dispatch with no pr_url, or a mention dispatch with no client_payload.pr_url) now share one slot, so a later sweep can cancel an in-flight one, and a dry-run can cancel a live sweep. Starting either needs write/dispatch access, so this is an availability trade-off, not a security one. Runs that name a PR keep their per-PR slot. It does not collide with the reusable's own pr-review-pr-* / pr-review-batch groups.
  3. ci-failure-analyst loop guard never matches (pre-existing). The if: excludes check runs whose name starts with CI Failure Analyst, but check-run names have the form <caller job> / <reusable job>, and the analyst's check was already named analyze / analyze before this PR (now ci-failure-analyst / analyze). So the guard was dead before and is still dead. A failing analyst run can therefore trigger another analyst run on the same commit. This is bounded by the job's per-commit, non-cancelling concurrency group and the reusable's non-fork filter. It is deliberately NOT changed here, to keep the collapse behavior-equivalent. A real guard is a separate follow-up, and the new bats test pins the current string.
  4. Check names change. review / review becomes pr-review / review and analyze / analyze becomes ci-failure-analyst / analyze. None of the five roles is a required check in this repo's ruleset, so no required-check edit is needed.
  5. Exposure is unchanged, not reduced. The consolidated workflow runs the same reusables with the same secrets on the same events, including comment and check events on a public repo, exactly as the five stubs did. What changes is that one file now holds all five if: filters, so an if: mistake affects the shared trigger union. The if: filters pass the org's pure-event-filter validator and are covered by the bats suite in this PR.
  6. Rollback. Revert this PR. That restores the five stubs byte-for-byte and removes agent-ingress.yml. The first live run of the collapsed workflow has not happened yet; it is observed after merge.