This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow-steward.git
The following commit(s) were added to refs/heads/main by this push:
new 2ae6e8d fix(pr-management-code-review): replace airflow paths with
adopter-config indirection (#67)
2ae6e8d is described below
commit 2ae6e8df770437b59e13af3cc5762d491ae7eecb
Author: André Ahlert <[email protected]>
AuthorDate: Wed May 6 12:04:38 2026 -0300
fix(pr-management-code-review): replace airflow paths with adopter-config
indirection (#67)
The skill embedded airflow-specific paths
(`.github/instructions/code-review.instructions.md`,
`providers/AGENTS.md`, `Cadwyn`) in 17 places. Adopters had no way to point
the skill at their
own review-criteria file.
Replace the hardcoded paths with the
`<project-config>/pr-management-code-review-criteria.md`
indirection the rest of the framework already uses. The existing template
`projects/_template/pr-management-code-review-criteria.md` serves as the
worked example.
The category list (Architecture boundaries, DB / query correctness, etc.)
is now an abstract
canonical list; concrete URLs come from the adopter's `Section anchors`
table. Backports section
is gated on the adopter declaring a backport branch pattern (no Cadwyn
wording). Provider-specific
section is now generic per-area subtree handling.
Validated with prek and skill-validator (0 violations).
---
.claude/skills/pr-management-code-review/SKILL.md | 26 +++--
.../pr-management-code-review/adversarial.md | 2 +-
.../skills/pr-management-code-review/criteria.md | 124 ++++++++++-----------
.../skills/pr-management-code-review/posting.md | 2 +-
.../pr-management-code-review/review-flow.md | 2 +-
5 files changed, 75 insertions(+), 81 deletions(-)
diff --git a/.claude/skills/pr-management-code-review/SKILL.md
b/.claude/skills/pr-management-code-review/SKILL.md
index 8939ef3..0495761 100644
--- a/.claude/skills/pr-management-code-review/SKILL.md
+++ b/.claude/skills/pr-management-code-review/SKILL.md
@@ -7,9 +7,11 @@ description: |
authenticated maintainer: PRs where review is requested from them, PRs that
touch files they recently modified, PRs
whose changed files they own per `CODEOWNERS`, PRs that `@`-mention them,
and PRs they already submitted a real
review on (triage comments do not count). Filters can narrow by area label,
collaborator status, or to a single PR.
- For each PR the skill reads the diff, applies the project's review criteria
-
([.github/instructions/code-review.instructions.md](../../../.github/instructions/code-review.instructions.md)
- and [AGENTS.md](../../../AGENTS.md)), runs any locally-configured
adversarial reviewer (e.g. the OpenAI
+ For each PR the skill reads the diff, applies the project's
+ review criteria (the source files declared in
+ `<project-config>/pr-management-code-review-criteria.md`, plus
+ the project's repo-wide `AGENTS.md`), runs any
+ locally-configured adversarial reviewer (e.g. the OpenAI
Codex plugin), surfaces findings, drafts an `approve` / `request-changes` /
`comment` review with
inline comments proposed by default, and — on the maintainer's confirmation
— posts it via the
`addPullRequestReview` mutation. This is the deep-review counterpart to the
triage skill.
@@ -155,9 +157,11 @@ proposing to invoke a locally-installed adversarial
reviewer.
**Golden rule 3 — criteria are authoritative; this skill is a
checker, not a re-interpreter.** The project's review criteria
-live in
-[`.github/instructions/code-review.instructions.md`](../../../.github/instructions/code-review.instructions.md)
-and [`AGENTS.md`](../../../AGENTS.md). When you find a violation,
+live in the source files declared in
+`<project-config>/pr-management-code-review-criteria.md` (see
+[`projects/_template/pr-management-code-review-criteria.md`](../../../projects/_template/pr-management-code-review-criteria.md)
+for the shape) and in the project's repo-wide
+[`AGENTS.md`](../../../AGENTS.md). When you find a violation,
quote the **specific rule** from those files in the review
finding. Do not invent new rules; do not soften documented ones.
A summary checklist lives in [`criteria.md`](criteria.md) for
@@ -534,11 +538,11 @@ writes a session log to disk.
[`posting.md`](posting.md), and never edits the contributor's
branch.
- **Bypassing the project's review criteria.** Findings cite
- specific rules from
-
[`.github/instructions/code-review.instructions.md`](../../../.github/instructions/code-review.instructions.md)
- and [`AGENTS.md`](../../../AGENTS.md). New review philosophies
- belong in those files first; this skill picks them up
- automatically once they land.
+ specific rules from the source files declared in
+ `<project-config>/pr-management-code-review-criteria.md` and
+ from the project's repo-wide [`AGENTS.md`](../../../AGENTS.md).
+ New review philosophies belong in those files first; this
+ skill picks them up automatically once they land.
---
diff --git a/.claude/skills/pr-management-code-review/adversarial.md
b/.claude/skills/pr-management-code-review/adversarial.md
index 6c694bd..47c85b6 100644
--- a/.claude/skills/pr-management-code-review/adversarial.md
+++ b/.claude/skills/pr-management-code-review/adversarial.md
@@ -129,7 +129,7 @@ source:
```yaml
- file: providers/foo/src/project/providers/foo/hook.py
line: 142
- rule_source: .github/instructions/code-review.instructions.md
+ rule_source: <project-config>/pr-management-code-review-criteria.md →
repo-wide source
rule_id: "Imports inside function bodies"
source: both # ← primary AND adversarial flagged this
severity: minor
diff --git a/.claude/skills/pr-management-code-review/criteria.md
b/.claude/skills/pr-management-code-review/criteria.md
index bbdd338..570141b 100644
--- a/.claude/skills/pr-management-code-review/criteria.md
+++ b/.claude/skills/pr-management-code-review/criteria.md
@@ -31,9 +31,9 @@ a concrete example.
| File | What it covers |
|---|---|
-| `.github/instructions/code-review.instructions.md` | Architecture / DB /
quality / testing / API / UI / generated files / AI-generated-code signals /
quality signals. |
+| `<repo_wide_review_criteria>` | The rule set every PR is reviewed against
(typically architecture / DB / quality / testing / API / UI / generated files /
AI-generated-code signals / quality signals). |
| `AGENTS.md` | Repo-wide AI/agent instructions (architecture boundaries,
security model, coding standards, testing standards, commits & PR conventions).
|
-| `<area>/AGENTS.md` | Per-area rules (e.g. `providers/AGENTS.md`,
`registry/AGENTS.md`, `dev/AGENTS.md`). |
+| `<area>/AGENTS.md` | Per-area rules (e.g. tree-specific or
subsystem-specific overlays). |
| `<security-model-doc>` | The documented security model — what *is* and
*isn't* a vulnerability. |
The per-PR review flow re-runs `git ls-files` against the
@@ -44,70 +44,52 @@ table; see
[`review-flow.md#area-specific-overlay`](review-flow.md).
## Categories — link out to the source section
-The headings below mirror the section structure of the source
-files; click through for the actual rule text.
-
-### Architecture boundaries
-
-[`code-review.instructions.md` § Architecture
Boundaries](../../../.github/instructions/code-review.instructions.md#architecture-boundaries)
·
-[`AGENTS.md` § Architecture
Boundaries](../../../AGENTS.md#architecture-boundaries)
-
-### Database / query correctness
-
-[`code-review.instructions.md` § Database and Query
Correctness](../../../.github/instructions/code-review.instructions.md#database-and-query-correctness)
-
-### Code quality
-
-[`code-review.instructions.md` § Code Quality
Rules](../../../.github/instructions/code-review.instructions.md#code-quality-rules)
·
-[`AGENTS.md` § Coding Standards](../../../AGENTS.md#coding-standards)
-
-### Testing
-
-[`code-review.instructions.md` § Testing
Requirements](../../../.github/instructions/code-review.instructions.md#testing-requirements)
·
-[`AGENTS.md` § Testing Standards](../../../AGENTS.md#testing-standards)
-
-### API correctness
-
-[`code-review.instructions.md` § API
Correctness](../../../.github/instructions/code-review.instructions.md#api-correctness)
-
-### UI (React/TypeScript)
-
-[`code-review.instructions.md` § UI Code
(React/TypeScript)](../../../.github/instructions/code-review.instructions.md#ui-code-reacttypescript)
-
-### Generated files
-
-[`code-review.instructions.md` § Generated
Files](../../../.github/instructions/code-review.instructions.md#generated-files)
-
-### AI-generated code signals
-
-[`code-review.instructions.md` § AI-Generated Code
Signals](../../../.github/instructions/code-review.instructions.md#ai-generated-code-signals)
-
-### Quality signals to check
-
-[`code-review.instructions.md` § Quality Signals to
Check](../../../.github/instructions/code-review.instructions.md#quality-signals-to-check)
-
-### Commits and PRs (newsfragments, commit messages, tracking issues)
-
-[`AGENTS.md` § Commits and PRs](../../../AGENTS.md#commits-and-prs)
+The category headings below are the **abstract names** the skill
+uses when grouping findings. The concrete URL each one links to
+lives in the adopter's
+`<project-config>/pr-management-code-review-criteria.md` →
+`Section anchors` table — every adopter substitutes their own
+review-doc URLs there. The skill resolves a category to its URL
+at finding time by matching the heading verbatim against the
+adopter table's `Section` column.
+
+If a category has no anchor row in the adopter config, the skill
+falls back to a plain reference (no clickable link) and surfaces
+the missing anchor as a one-line warning at the top of the
+review.
+
+The canonical category list:
+
+- Architecture boundaries
+- Database / query correctness
+- Code quality
+- Testing
+- API correctness
+- UI (React/TypeScript)
+- Generated files
+- AI-generated code signals
+- Quality signals to check
+- Commits and PRs (newsfragments, commit messages, tracking issues)
+- Security model
+
+See
+[`projects/_template/pr-management-code-review-criteria.md` § Section
anchors](../../../projects/_template/pr-management-code-review-criteria.md#section-anchors)
+for a worked example.
---
-## Provider-specific signals
+## Per-area / subtree-specific signals
-When a PR touches `providers/<name>/`, the skill reads (and
-quotes from) the provider-tree files in addition to the
-repo-wide ones:
-
-- [`providers/AGENTS.md`](../../../providers/AGENTS.md) — the
- provider-boundary, compat-layer, and `provider.yaml`
- expectations apply.
-- `providers/<name>/AGENTS.md` if present — provider-specific
- rules (e.g.
-
[`providers/elasticsearch/AGENTS.md`](../../../providers/elasticsearch/AGENTS.md),
- [`providers/opensearch/AGENTS.md`](../../../providers/opensearch/AGENTS.md)).
+When a PR touches a subtree the adopter listed in
+`<project-config>/pr-management-code-review-criteria.md` →
+`Per-area source files`, the skill reads (and quotes from) those
+per-area files in addition to the repo-wide ones. The skill also
+auto-discovers any `AGENTS.md` under the touched paths via
+`git ls-files`, so an `AGENTS.md` not listed in the table is
+still loaded if the diff touches its tree.
-If the provider's tree has no `AGENTS.md`, the repo-wide rules
-are still in effect.
+If a touched subtree has no `AGENTS.md` and no entry in the
+adopter table, only the repo-wide rules apply.
---
@@ -137,21 +119,29 @@ calibration explicitly. Don't paraphrase.
## Backports and version-specific PRs
-Branch `vX-Y-test` PRs are backports of already-merged `main`
-work. They aren't called out in the repo-wide files, so the
-calibration is local to this skill:
+If the adopter's
+`<project-config>/pr-management-code-review-criteria.md` declares
+a backport branch pattern (see its `Backports / version-specific
+PRs` table), PRs whose base branch matches the pattern are
+treated as backports of already-merged `main` work and get a
+lighter-touch calibration:
- **Diff parity**: does this match what was merged on `main`?
- **Cherry-pick conflicts**: did the resolution introduce new
changes that need scrutiny?
- **API/migration version markers**: backports should not
- introduce new Cadwyn version bumps; if they do, that's a
- finding (cite
- [`code-review.instructions.md` § API
Correctness](../../../.github/instructions/code-review.instructions.md#api-correctness)).
+ introduce new version bumps in any
+ versioning-sensitive subsystem the adopter calls out; if they
+ do, cite the relevant `API correctness` anchor from the
+ adopter's `Section anchors` table.
For these PRs, prefer `COMMENT` over `REQUEST_CHANGES` unless
the cherry-pick has clearly drifted from the `main` change.
+If the adopter config has no backport pattern declared, this
+section is a no-op and every PR is reviewed under the same
+calibration.
+
---
## Conflict between source rules
diff --git a/.claude/skills/pr-management-code-review/posting.md
b/.claude/skills/pr-management-code-review/posting.md
index bd16dae..04cf67d 100644
--- a/.claude/skills/pr-management-code-review/posting.md
+++ b/.claude/skills/pr-management-code-review/posting.md
@@ -217,7 +217,7 @@ For each `blocking` finding:
````markdown
### Blocking — [short rule name] (`file.py:142`)
-> [verbatim quote of the rule from
.github/instructions/code-review.instructions.md or AGENTS.md]
+> [verbatim quote of the rule from one of the source files declared in
`<project-config>/pr-management-code-review-criteria.md`]
```text
[5–10 lines of context from the diff, with a `# ←` arrow at the offending line]
diff --git a/.claude/skills/pr-management-code-review/review-flow.md
b/.claude/skills/pr-management-code-review/review-flow.md
index 5f160fa..d5129eb 100644
--- a/.claude/skills/pr-management-code-review/review-flow.md
+++ b/.claude/skills/pr-management-code-review/review-flow.md
@@ -185,7 +185,7 @@ For each finding, record:
```yaml
- file: providers/foo/src/project/providers/foo/hook.py
line: 142
- rule_source: .github/instructions/code-review.instructions.md
+ rule_source: <project-config>/pr-management-code-review-criteria.md →
repo-wide source
rule_section: "#code-quality-rules"
rule_id: |
a short identifier copied verbatim from the source rule
