justinmclean commented on PR #147:
URL: https://github.com/apache/airflow-steward/pull/147#issuecomment-4569635521

   There still a couple of ASF policy issues:
   
   1. Amendment vote model is mislabeled
   PR description claims: the amendment process matches the release-vote 
process (>=3 binding +1, no binding -1, 72h, no lazy consensus).
   ASF voting policy actually says:
   
   Release votes use majority approval: >=3 binding +1, more positive than 
negative binding, releases cannot be vetoed.
   Code-modification votes use consensus approval: >=3 binding +1, any binding 
-1 is a veto that stops the change until withdrawn.
   
   The rule encoded in PRINCIPLES.md (>=3 binding +1, any binding -1 vetoes) is 
the code-modification model, not the release model. The chosen rule is correct 
for governance changes. The PR narrative and the doc header just describe it 
with the wrong label.
   
   2. Veto-justification requirement is missing
   ASF voting policy: "A veto without a justification is invalid and has no 
weight. To prevent vetoes from being used capriciously, the voter must provide 
with the veto a technical justification."
   PRINCIPLES.md opening rule says any committer may block on principle 
grounds, and the amendment section says a binding -1 stops the amendment until 
withdrawn. Neither requires a technical justification.
   This is a real gap. Without it, a committer could -1 any change citing a 
principle without explaining how the change actually violates the principle, 
and the change is stuck. ASF policy would treat such a -1 as invalid.
   
   3. Generative tooling disclosure is missing from P17
   ASF Generative Tooling Guidance: "When providing contributions authored 
using generative AI tooling, a recommended practice is for contributors to 
indicate the tooling used to create the contribution. This should be included 
as a token in the source control commit message, for example including the 
phrase 'Generated-by: <tool>'."
   PRINCIPLES.md P17 says contributions land under Apache-2.0 and incompatible 
dependencies do not enter the framework. It does not mention AI-disclosure at 
all.
   
   The rest looks good to me. Running this over our existing skills gave me 
this. Do we want to revisit the P14 size limit?
   
   # Audit: existing skills vs PRINCIPLES.md (PR #147)
   
   Date: 2026-05-29
   Scope: 30 skills under `.claude/skills/`, checked against the 19 principles 
in `PRINCIPLES.md` from PR #147.
   
   Two principles are the real gates: **P14 (size + structure)** and **P3 + P12 
(non-ASF first-class + placeholder hygiene)**. P0, P6, P8, and P14's 
sibling-link rule are largely already satisfied.
   
   ## P14 size violations (the ≤500-line rule)
   
   13 of 30 SKILL.md files are over. Ranked worst first, with how much they 
overshoot:
   
   | Skill | Lines | Over by |
   |---|---|---|
   | security-issue-sync | 3060 | 6.1x |
   | security-issue-import | 1841 | 3.7x |
   | security-issue-triage | 1057 | 2.1x |
   | security-issue-fix | 946 | 1.9x |
   | security-issue-invalidate | 874 | 1.7x |
   | security-issue-import-from-pr | 816 | 1.6x |
   | security-issue-import-from-md | 797 | 1.6x |
   | pr-management-triage | 761 | 1.5x |
   | issue-triage | 737 | 1.5x |
   | security-cve-allocate | 737 | 1.5x |
   | security-issue-deduplicate | 620 | 1.2x |
   | pr-management-code-review | 596 | 1.2x |
   | issue-reproducer | 524 | barely |
   
   Pattern: every `security-*` skill except the dashboard is over. So is the 
entire triage family. `security-issue-sync` and `security-issue-import` are the 
structural outliers. Both are flat (zero siblings), so the fix is "pull 
reference material out into linked sibling markdown", which is exactly what 
`pr-management-triage` already does (10 siblings) and 
`pr-management-code-review` does (6 siblings) even though both still overshoot.
   
   `issue-reproducer` at 524 is one inline section away from compliance. 
Cheapest landing.
   
   The sibling-depth rule is **clean** everywhere: nothing is nested more than 
one level, and every existing sibling is linked from SKILL.md.
   
   ## P3 + P12: non-ASF adopters and placeholder hygiene
   
   This is the substantive gap, and it is bigger than the size cliff.
   
   The entire **security-issue-\*** family is implicitly ASF-only. 
`security-issue-import/SKILL.md` hard-codes `[email protected]` as the relay 
address, the ASF forwarding preamble as the load-bearing signal, and 
`cveprocess.apache.org/cve5/...` as the CVE-tool surface. P3 says non-ASF 
adopters are first-class, P12 says concrete ASF infra inside `.claude/skills/` 
is a refactor bug, and these two principles collide head-on in this skill. 
Options:
   
   - Add a `<security-relay>` / `<cve-tool>` placeholder layer in adopter 
config and resolve at runtime, or
   - Scope the security family explicitly as ASF-only and own the carve-out in 
the skill description.
   
   Either is fine. Pretending the current state is project-agnostic is not. 
`security-issue-sync`, `security-issue-import`, `security-issue-triage`, 
`security-issue-import-from-pr`, `security-issue-import-from-md`, 
`security-cve-allocate`, and `security-issue-invalidate` all carry the same 
coupling.
   
   `setup-steward` references `apache/airflow-steward` 8 times, but those are 
legitimate self-references to the framework's own repo. Not a violation, but 
the principle text would benefit from naming the carve-out explicitly. Same for 
`pr-management-triage/comment-templates.md` falling back to 
`[email protected]` in templates.
   
   ## What's already fine
   
   - **P8 (eval as release blocker)**: every one of the 30 skills has a 
matching directory under `tools/skill-evals/evals/`, with case counts ranging 
from 1 to 11. No gaps.
   - **P0 (external content as data)**: every skill that ingests external 
content names the rule. The 10 skills without P0 language are 
setup/install/sync/dashboard skills that do not process external content.
   - **P14 sibling structure**: where siblings exist, they are linked from 
SKILL.md; no orphans, no nesting beyond one level.
   - **P6 (human sign-off on outbound communication)**: covered in practice by 
skills that draft comments and require review before posting. The wording is 
not the wording P6 uses, but the behavior is correct. Documentation alignment, 
not a behavioral fix.
   
   
   


-- 
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]

Reply via email to