docs: align AGENTS.md and contributing/SKILL.md on skill-update gate - #862
mrbobbytables wants to merge 1 commit into
Conversation
Clarify across AGENTS.md, docs/SKILL.md, contributing/SKILL.md, and skill-improvement/SKILL.md that the skill-update gate is a mandatory review gate before enqueuing, triggered when changes introduce or alter a pattern, workaround, or contract. Pure refactorings, helper deduplication, and added test coverage without new conventions are explicitly exempt. Also clarify that when no existing skill page directly covers an area, contributors should update the closest matching skill or create one under docs/skills/<category>/<area>/SKILL.md. Fixes projectbluefin#861 Assisted-by: Gemini 3.8 Flash via GitHub Copilot CLI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: mrbobbytables <mrbobbytables@users.noreply.github.com>
castrojo
left a comment
There was a problem hiding this comment.
Backlog triage pass. I have not approved and have not merged or enqueued anything: .github/copilot-instructions.md says agents never approve, merge, or enqueue, and AGENTS.md:136,163 + docs/skills/meta/human-gates/SKILL.md make merge a human gate. This is a review comment only.
This resolves #861, which I filed earlier today during this backlog pass, and it resolves it correctly. Reviewed the diff rather than the description.
The contradiction is genuinely gone. Before, AGENTS.md:146 listed the skill update under ## Mandatory gates before enqueuing any PR while docs/skills/ci-ops/contributing/SKILL.md:78 called the same rule something that "should" happen and was "a review expectation, not an automated check". An agent reading the router got a different answer depending on which file it hit first. After this, all four files say the same thing: it is a mandatory review gate, enforced by reviewers before enqueuing rather than by CI, and it triggers on introducing or altering a pattern, workaround, or contract.
It took option 1 from the issue, which is the better of the two. Narrowing the trigger rather than downgrading the rule keeps the gate meaningful where it earns its place and stops it generating doc noise where it does not. The three exempt categories — pure refactor, helper dedup, added coverage without new conventions — are exactly the cases that motivated the issue, and I can confirm they map onto real PRs in the current backlog: #787 folds two byte-identical copies of a private _first_value helper into tests/shared/ssh_config.py, and #825 routes the AT-SPI dump through an existing resolve_results_dir. Neither introduces a convention, and under the old wording both would have had to grow a paragraph whose only purpose was satisfying a checkbox — which AGENTS.md:75 bans as "append here" docs.
The closest-skill fallback ("update the closest matching skill or create one under docs/skills/<area>/SKILL.md") also answers the open question I raised in the issue about what "matching" means when no page covers the subject.
Two things I checked specifically:
- The
skill-improvement/SKILL.mdchecklist rewrite is a real improvement, not just a reword: turning "Did I discover any workaround..." into an explicit yes/no branch with the exempt outcome stated first means the answer is reachable without reading the whole page. - Removing the stale
skill-drift.mdreference and the retired-in-#681 drift check is correct — that file does not exist in the tree.
One thing this needs that the green checks do not cover: it edits AGENTS.md, and docs/skills/meta/human-gates/SKILL.md:130-132 requires a recorded human lgtm for PRs touching AGENTS.md, CODEOWNERS, or e2e.yml. That is on top of the two approvals — this is a behavioural directive change for every agent in the repo, so it should get a real read rather than a rubber stamp.
For transparency about my own position: I am the one who filed #861, and while it was open I treated the rule as contested and flagged the gap in reviews rather than manufacturing skill notes to satisfy it. Under this PR's wording those calls hold — #787 and #825 are exempt — and where a PR did introduce a new contract I wrote the note rather than flagging it (#813's deny-all permissions default, #816's screenshot fallback chain). So this change ratifies the line I was already drawing, which is a reason for someone other than me to check it.
All six checks pass on 0b01ebcc. ghost-lab has not posted for this head yet; it is red or unposted across the entire backlog and every other repo in the factory, which I have documented separately.
Gate status for this PR
- Ruleset
main — merge queuerequires 2 approving reviews (required_approving_review_count: 2,dismiss_stale_reviews_on_push: true,require_last_push_approval: true, andbypass_actorsis empty — nobody can bypass). Required checks:Lint & syntax,Behave dry-run,pytest. ghost-labis red, and that is not this PR's fault. Every lab workflow in the Argo namespace isFailed/Errorsince 2026-09-15 acrosstestsuite,common,knuckleandbluefin-lts; the last success anywhere was testsuite #790 on 2026-09-15.docs/skills/meta/human-gates/SKILL.md:80-107still makesghost-laba merge gate, so I am naming it as outstanding rather than waiving it.
Resolves #861.
Problem
AGENTS.mdanddocs/skills/ci-ops/contributing/SKILL.mdpreviously had contradictory guidance on whether updating a skill file was a mandatory gate or an unenforced review expectation for changes undertests/**,.github/workflows/**, orscripts/**. Additionally, a blanket requirement led to triage confusion for PRs that purely refactor, deduplicate helpers, or add unit test coverage without introducing or altering any pattern, workaround, or contract.Solution
AGENTS.md: Kept the gate in## Mandatory gates before enqueuing any PR, but narrowed the trigger: a matching skill update is required if the change introduces or alters a pattern, workaround, or contract. Pure refactorings, helper deduplications, and test coverage additions without new conventions are explicitly exempt. Clarified that if no skill file covers the area, contributors should update the closest matching skill or create one underdocs/skills/<area>/SKILL.md.docs/skills/ci-ops/contributing/SKILL.md: Updated the skill-improvement mandate description to matchAGENTS.md(mandatory review gate for new/altered patterns, workarounds, or contracts; pure refactors/dedups/coverage exempt; closest-skill fallback) and clarified that it is enforced during review prior to enqueueing rather than by an automated CI job.docs/skills/meta/skill-improvement/SKILL.md: Aligned the pre-enqueuing checklist and guidance with the narrowed trigger, added pure refactoring/dedup/coverage to the exempt categories table, and removed stale references to the retired skill-drift check and non-existentskill-drift.md.docs/SKILL.md: Updated hard rule 10 to reflect the narrowed trigger and exemption for pure refactors/coverage additions.— hive: backend=copilot model=gemini-3.8-flash
🐝 Hive Agent:
contributor| SHA:0b01ebcc