feat(review): require repository review-skill readiness - #608
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Resolve the manifest routing and validation gaps, and correct the hidden-directory installation command.
Pull request overview
Adds mandatory repository review-skill readiness checks and a canonical portable code-review skill.
Changes:
- Updates policy and health/bootstrap runbooks.
- Adds the portable review skill and installation guidance.
- Registers skill and review-readiness routing in the manifest.
File summaries
| File | Summary |
|---|---|
runbooks/recurring-operations.md |
Extends recurring health checks. |
runbooks/org-review.md |
Defines readiness and installation procedures. Nit (1 vote): add --allow-hidden-dirs to the documented CLI command. |
runbooks/new-repository.md |
Adds skill delivery to repository bootstrap. |
PATTERNS.md |
Documents the maintained skill-copy exception. |
AGENTS.md |
Adds the mandatory readiness requirement. |
.github/skills/code-review/SKILL.md |
Adds portable review guidance. Moderate (1 vote): add or wire in skill-specific validation and regression tests. |
.github/instruction-surfaces.json |
Registers routing metadata. Moderate (1 vote each): add review-readiness routing to the required surface and advisory skill, and add regression coverage for the new routing and policy requirements. |
Review details
Suppressed comments (5)
.github/instruction-surfaces.json:544
- The manifest introduces
review-readinessas a canonical domain here, but this required surface'staskslist has noreview-readinessentry, and no other surface routes that task. A runtime selecting work by that task therefore cannot discover the required readiness procedure. Add the task to this surface (and keep the advisory skill route consistent).
"canonical_for": ["organization-review", "review-readiness"]
.github/instruction-surfaces.json:316
- This advisory skill describes review-readiness work, but its manifest task list omits the new
review-readinesstask. On runtimes that route skills by task, an explicit review-readiness evaluation will not discover this skill even though the runbook declares that domain. Addreview-readinessto this list.
"repository-health-check"
.github/instruction-surfaces.json:309
- The new readiness contract is not protected by the repository's tests:
validate-agent-policy.pydiscovers any existing skill and validates manifest shape, but it does not requireskill-code-reviewor assert theAGENTS.md/runbook-org-reviewrouting. Removing this surface would therefore still leavetest_public_repository_has_no_validation_errorsgreen. Add a regression test that asserts the new skill entry, its health-task routing, and the required review-readiness policy/runbook text.
"id": "skill-code-review",
"path": ".github/skills/code-review/SKILL.md",
"kind": "skill",
"authority": "advisory",
"consumers": ["codex", "claude-code", "copilot", "gemini-cli", "human"],
.github/skills/code-review/SKILL.md:4
- The new readiness baseline is not protected by the repository's required validator:
validate-agent-policy.pyinventories skill files and scans their text for forbidden tokens, but does not validate this skill's frontmatter, requiredcode-reviewname/description, actionable body, or links—the dimensions thatrunbooks/org-review.mdsays determine validity. A later malformed or broken skill could therefore pass the stated policy validation and still be reported ready. Add a skill-specific validator and regression tests, or wire the existing skill validation into CI.
---
name: code-review
description: Review pull requests, diffs, and code changes using repository contracts and checks, or assess review readiness during repository-health evaluations. Produce evidence-based findings without authorizing fixes or external writes.
---
runbooks/org-review.md:106
- The installation source is under the hidden
.githubdirectory, but the GitHub CLI skill-install contract requires--allow-hidden-dirswhen traversing dot-directories. As written, this documented bootstrap command can fail before it installs the required.github/skills/code-review/SKILL.md; include the hidden-directory flag (and verify it with the installed CLI as instructed above).
gh skill install z-shell/.github .github/skills/code-review --pin <approved-commit-sha> --dir .github/skills
- Files reviewed: 8/8 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved validation-contract and recurring-readiness findings remain.
Review details
Suppressed comments (4)
.github/instruction-surfaces.json:309
- This adds the readiness skill only as a generic manifest entry;
validate-agent-policy.pydiscovers skill surfaces that exist but does not require the specificskill-code-reviewentry or file. Removing both would leave the policy validator green, so the canonical readiness source could disappear without detection. Add an exact public-policy contract test or validator invariant for this surface, including its routes and advisory status.
"id": "skill-code-review",
"path": ".github/skills/code-review/SKILL.md",
"kind": "skill",
"authority": "advisory",
"consumers": ["codex", "claude-code", "copilot", "gemini-cli", "human"],
.github/skills/code-review/SKILL.md:3
- This is the canonical skill that the new health gate asks reviewers to validate, but the repository validator only checks that the manifest path is a regular file and scans resources; it does not validate the required frontmatter (
name: code-review, non-empty review description) or actionable body, and the public tests only assert no validator errors. A future malformed skill could therefore pass CI while failing the documented readiness contract. Add a focused validator/test for this metadata contract (or make the readiness check executable).
name: code-review
description: Review pull requests, diffs, and code changes using repository contracts and checks, or assess review readiness during repository-health evaluations. Produce evidence-based findings without authorizing fixes or external writes.
AGENTS.md:128
- This new mandatory gate is not pinned by a regression test. The current validator discovers whichever
.github/skills/*/SKILL.mdfiles exist and only checks that discovered paths are declared, so removing this skill and its manifest entry would still let the generic validation pass. Add a public-repository test that asserts thecode-reviewskill declaration and the health/bootstrap routing and policy requirement, so the readiness gate cannot be silently removed.
Every repository-health evaluation, including quick checks and bootstrap,
must assess `.github/skills/code-review/SKILL.md` for presence, validity,
source currency, local drift, and suitability for the repository. Follow
`runbooks/org-review.md`; report unassessed or unavailable evidence explicitly.
Missing or unsuitable guidance prevents a clean review-readiness result.
runbooks/recurring-operations.md:34
- The canonical readiness procedure treats invalid, stale, modified, and unverified results as reportable failed dimensions, but this recurring-operations rule asks reviewers to record only missing or unsuitable skills. A quick recurring health review can therefore omit other readiness failures from remediation findings. Record every failed dimension listed by org-review.md, with evidence and an owner.
including quick evaluations. Record missing or unsuitable review skills as
remediation findings under the existing owner. The evaluation does not
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Lite
Agent handoffStatus: Ready for maintainer review Current stateThe canonical skill, mandatory health policy, explicit review-readiness routing, and focused public validation are published. Copilot's confirmed metadata and routing gaps are fixed. Recurring operations now report every failed or unverified readiness dimension. Verification
Next steps
|
Repository-health reviews currently have no required check that a repository can receive appropriate code-review guidance. This change makes review-skill readiness part of every health evaluation, including quick checks and repository bootstrap, and adds the canonical portable
.github/skills/code-review/SKILL.md.The skill discovers repository contracts and validation commands, applies relevant checks for shell plugins and annexes, Go, compiled modules, documentation, packaging, and infrastructure, and produces evidence-based findings. Reviews remain read-only unless remediation is authorized. Existing code-review instructions remain the canonical review criteria.
The health procedure checks presence, validity, provenance, source drift, and repository suitability. It distinguishes local drafts from published coverage and static readiness from observed runtime invocation. Maintained repository-local copies are an explicit exception to the generic skill-duplication restriction; delivery uses an approved published commit and an explicit
.github/skillsdestination. Pinned copies require explicit source comparison becausegh skill update --dry-runskips them.Closes #607. Refs #606 for rollout and #485 for recurring-operations coordination. Organization-wide delivery and hosted Copilot invocation are subsequent rollout steps, not established by this policy PR.
Instruction-impact review
AGENTS.md;runbooks/org-review.mdowns the procedure. The new skill is advisory runtime guidance. Bootstrap, recurring-operation, and placement documentation now reference that procedure. The existing public validator now enforces canonical skill presence, its name/description/body contract, and regression coverage protects mandatory/advisory routing.z-shell/.githubowns the shared skill and health procedure..github/instructions/code-review-generic.instructions.mdcontinues to own review criteria; each consuming repository owns its specific contracts and verification commands.org-review.mdcanonical ownership of review readiness, and routes that required procedure to repository bootstrap and the explicitreview-readinesstask. Consuming instruction manifests and generated deliveries must receive the corresponding baseline and skill update during rollout.AGENTS.mdstates the requirement directly and links to the required, manifest-routed runbook. Skill discovery and actual invocation remain separate evidence; neither is necessary to receive the health rule.Validation
Run from this repository's root:
All passed: 88 agent-policy tests, 17 decision-record tests, 99 Zsh-standard tests, all three validators, and whitespace validation. Supplemental skill-creator validation also passed. Local Markdown targets and anchors were checked, and newly authored text contains no U+2014 characters.
The frozen consumer-parser digest was updated for the reviewed
PATTERNS.mddelivery exception, as the test contract requires. Comparing the parent and current snapshots confirmed that only parsedPATTERNS.mdoutput changed; all 66 rule blocks, parsed rules, the documentation registry, and other consumer outputs remained identical.Hosted Copilot reviews completed. Confirmed metadata-validation and explicit task-alias gaps were addressed; recurring operations now report every failed or unverified readiness dimension. The proposed hidden-directory flag is unnecessary for the documented exact-path install, which succeeded repeatedly with GitHub CLI 2.100.0. Actual Copilot skill attribution remains unverified. The first published source revision enables pinned delivery; file presence alone does not demonstrate that a review runtime used the skill.