justinmclean opened a new pull request, #187: URL: https://github.com/apache/airflow-steward/pull/187
## Problem The `pr-management-code-review` skill had no "License headers" category. A PR that added a new source file without the standard Apache license header would pass the skill's Step 4 scan entirely unchecked — no finding, no `REQUEST_CHANGES` disposition. This was confirmed by cross-checking the skill's category list against the ASF source headers policy (`https://www.apache.org/legal/src-headers.html`), which requires every ASF source file to carry the standard Apache header. The requirement existed in ASF policy but had no corresponding check in the review framework. As a secondary finding during the fix, `review-flow.md` Step 4's hardcoded category list was also out of sync with `criteria.md` — it enumerated 8 categories while `criteria.md` listed 11. The missing three ("Quality signals to check", "Commits and PRs", "Security model") have been added in the same pass. ## Changes **`.claude/skills/pr-management-code-review/criteria.md`** - Added `License headers` to the canonical category list (between "Code quality" and "Testing"), matching the ordering in `review-flow.md`. **`.claude/skills/pr-management-code-review/review-flow.md`** - Expanded Step 4's hardcoded category enumeration from 8 to 13 entries to match `criteria.md`. - "License headers" is now category #4. The three previously missing categories (10, 11, 12) are also added. **`projects/_template/pr-management-code-review-criteria.md`** - Added a "License headers" row to the Section anchors table with the ASF policy URL (`https://www.apache.org/legal/src-headers.html`) as the default anchor. - This is the URL the skill resolves at finding time to link contributors directly to the policy. Because this is a global ASF requirement — not project-specific — the URL ships as a ready-to-use default rather than a blank placeholder, so adopters don't need to fill it in. ## Testing **Structural validation** The `tools/skill-validator` suite (`test_real_repo_passes`) was run against all `SKILL.md` files post-change. Result: 1 passed, 0 failed. **Functional dry-run against synthetic PR content** The skill was executed in `dry-run` mode against a hand-constructed PR diff: - PR adds two new files: `airflow/utils/dag_bag_utils.py` (missing license header) and `tests/utils/test_dag_bag_utils.py` (header present and correct). - Step 4 walked all 13 categories. At category #4 ("License headers"), the skill detected the missing header on `dag_bag_utils.py:1`, raised a `major` finding citing `https://www.apache.org/legal/src-headers.html`, and generated a `suggestion` block with the correct header text. - The test file with the header present produced no finding (no false positive). - Disposition auto-picked `REQUEST_CHANGES` (1 major finding). - The draft review body included the finding with verbatim rule quote, diff excerpt with arrow at the offending line, and a GitHub `suggestion` block the contributor can apply in one click. - Before this fix, the same diff produced zero findings under this category because the category did not exist. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
