Skip to content

feat(review): require repository review-skill readiness - #608

Merged
ss-o merged 3 commits into
mainfrom
feature-607-review-skill-readiness
Sep 11, 2026
Merged

feat(review): require repository review-skill readiness#608
ss-o merged 3 commits into
mainfrom
feature-607-review-skill-readiness

Conversation

@ss-o

@ss-o ss-o commented Sep 11, 2026

Copy link
Copy Markdown
Member

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/skills destination. Pinned copies require explicit source comparison because gh skill update --dry-run skips 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

  1. Shared policy, scoped guidance, runtime behavior, or enforcement? The mandatory health requirement is shared policy in AGENTS.md; runbooks/org-review.md owns 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.
  2. Which runtimes and repository contexts receive it? Codex, Claude Code, Copilot, Gemini CLI, and human reviewers receive the mandatory baseline and routed health procedure. The skill supports standalone organization repositories and maintained forks using their existing local contracts. Archived or inaccessible repositories remain explicitly identified in coverage reports.
  3. Is the canonical owner correct? Yes. z-shell/.github owns the shared skill and health procedure. .github/instructions/code-review-generic.instructions.md continues to own review criteria; each consuming repository owns its specific contracts and verification commands.
  4. Does another surface duplicate or contradict it? The placement restriction now explicitly permits maintained review-skill delivery copies. Bootstrap and recurring-operation guidance link to the health procedure instead of defining separate criteria. The skill preserves read-only boundaries and does not become a policy owner.
  5. Do manifests need changed routes? The public manifest declares the advisory skill for code-review and health tasks, gives org-review.md canonical ownership of review readiness, and routes that required procedure to repository bootstrap and the explicit review-readiness task. Consuming instruction manifests and generated deliveries must receive the corresponding baseline and skill update during rollout.
  6. Can mandatory policy reach every runtime without an optional skill? Yes. AGENTS.md states 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.
  7. Do generated output and size limits pass? The public policy validator passes, including the policy size limit, inventory, routing, and privacy checks. This PR changes no generated public output. Any downstream generated delivery must be regenerated and validated by its owner before claiming rollout completion.

Validation

Run from this repository's root:

python3 scripts/validate-agent-policy.py
python3 -m unittest scripts/test_validate_agent_policy.py -v
python3 -m unittest scripts/test_decision_records.py -v
python3 scripts/decision-records.py --check
python3 -m unittest scripts/test_validate_zsh_standard_policy.py -v
python3 scripts/validate-zsh-standard-policy.py
git diff --check

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.md delivery exception, as the test contract requires. Comparing the parent and current snapshots confirmed that only parsed PATTERNS.md output 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.

@ss-o
ss-o requested a review from a team as a code owner September 11, 2026 12:08
Copilot AI lite review requested due to automatic review settings September 11, 2026 12:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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-readiness as a canonical domain here, but this required surface's tasks list has no review-readiness entry, 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-readiness task. On runtimes that route skills by task, an explicit review-readiness evaluation will not discover this skill even though the runbook declares that domain. Add review-readiness to 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.py discovers any existing skill and validates manifest shape, but it does not require skill-code-review or assert the AGENTS.md/runbook-org-review routing. Removing this surface would therefore still leave test_public_repository_has_no_validation_errors green. 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.py inventories skill files and scans their text for forbidden tokens, but does not validate this skill's frontmatter, required code-review name/description, actionable body, or links—the dimensions that runbooks/org-review.md says 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 .github directory, but the GitHub CLI skill-install contract requires --allow-hidden-dirs when 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.

Copilot AI review requested due to automatic review settings September 11, 2026 12:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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.py discovers skill surfaces that exist but does not require the specific skill-code-review entry 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.md files 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 the code-review skill 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

Copilot AI review requested due to automatic review settings September 11, 2026 12:27
@ss-o

ss-o commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Agent handoff

Status: Ready for maintainer review
Repository: z-shell/.github
Branch/PR: #608, commit dcaeaf086ae4ce8743fa69c84304c6d9cb99d526
Tracker/Issue: #607; rollout #606

Current state

The 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

  • 88 agent-policy, 17 decision-record, and 99 Zsh-standard tests passed locally, with all three validators and relevant Trunk checks.
  • Hosted checks passed on the current commit.
  • Skill bytes remain SHA-256 2b4bdb7d716ce91d8d63cb97526cb404407fcd333bf005165c4007ab38808998; both pilot installs retain approved source pin db20047dd6f9aba014fadc8152b60452f20ef037.
  • The documented exact-path native install succeeded repeatedly with GitHub CLI 2.100.0 without --allow-hidden-dirs. That suppressed suggestion was not applied.
  • Copilot completed reviews; returned comments do not expose explicit skill attribution, so invocation remains unverified.

Next steps

  1. Complete maintainer review and obtain merge approval for this canonical PR.
  2. Verify the new runbook anchor on main, then unblock wiki #915 and zsh-fancy-completions build(deps): bump dessant/lock-threads from 3.0.0 to 4.0.0 #65.
  3. Resolve runtime-attribution evidence before wider rollout. Keep feat(review): require repository review-skill readiness #606 open for remaining organization delivery.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Broad policy, routing, and runtime-readiness changes warrant final human review.

Review details
  • Files reviewed: 10/10 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@ss-o
ss-o enabled auto-merge (squash) September 11, 2026 12:42
@ss-o
ss-o disabled auto-merge September 11, 2026 12:43
@ss-o
ss-o merged commit 1088c0f into main Sep 11, 2026
15 of 16 checks passed
@ss-o
ss-o deleted the feature-607-review-skill-readiness branch September 11, 2026 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(review): add canonical skill and health-readiness policy

2 participants