code-review

Files (1)

• SKILL.md

SKILL.md (first lines)

# Project Code Review

Public skills in this repository are instructions executed verbatim by LLM
agents of varying capability. Review behavior, not prose: the question is
never "is this well written" but "which reader, on what input, does the
wrong thing". This file is also read by GitHub Copilot code review from its
git path; every rule below applies to any reviewer, human or machine.

## Baseline procedure

1. **Resolve the target.** Default is the current branch's diff against the
   main branch (staged and unstaged included); a PR number, branch, or path
   argument overrides it.
2. **Calibrate effort.** `low`/`medium`: fewer, high-confidence findings
   only. `high` and above: broader coverage; uncertain findings allowed but
   marked as such. When no level is given, reuse the last one used.
3. **Find, verify, report.** Draft candidate findings, verify each one
   (see "Verifying findings"), then report exactly once: through the
   host's typed findings tool when the harness provides one — ranked most
   severe first, empty if nothing survived, never duplicated as prose —
   otherwise as a ranked list.
4. **Arguments.** `--fix`: after reporting, apply the surviving findings to
   the working tree. `--comment`: post findings as inline PR comments — a
   remote write, so `agent-authority.md ## Publishing` applies to the
   exact comment payloads.
5. A deep multi-agent cloud review ("ultra") is a host feature, not part of
   this skill; when it is unavailable, run at maximum effort locally and
   say that is what happened.

## Severity ladder

1. **Critical — leaked secrets, private data, or content that must not be
   committed.** Always the top of the report, at every effort level.
2. **Blocking — wrong installed behavior**: an agent following a published
   skill's text takes a wrong or unsafe action; agent authority widened by
   default (ready/review/merge granted on green checks, a weakened
   no-self-escalation rule); a builder-defined model noun inside a
   deposited platform file, or a platform object decided during the
   platform-neutral design stage; a contradiction between pieces that load
   together; a breach of
   `skill-quality.md ## Self-containment and dependencies`.
3. **Recommended**: a latent ambiguity with a plausible failure path; a
   harness desync (see the quality gradient).
4. **Note**: grammar, terminology, phrasing. Never marked blocking, and
   never proposed in a way that violates the constraint surface below.

## Pass one, on every review: sensitive and misplaced content

Before any quality finding, re-examine the full added content — prose,
fixtures, and data included, not just code — for material that should not
be committed:

- Credentials of any kind: tokens, API keys, private keys, passwords,
  signed URLs, cloud account identifiers.
- Private or personal data: real names and emails outside intended
  authorship metadata, internal hostnames or URLs, customer or user data,
  anything pasted from a private conversation.
- Content that belongs outside version control: session transcripts, eval
  fixtures and outputs, personal harness configuration
  (`.claude/settings.local.json` and similar; the shared
  `.claude/settings.json` allowlist is tracked on purpose), scratch files,
  large generated artifacts.

The pre-commit hooks (gitleaks, detect-secrets) catch known token patterns;
this pass hunts what pattern matching misses — novel token formats, secrets
inside prose or test data, private information that is not a credential.
When the diff carries risky material, run the `sensitivity-check` skill over
it for the deep scan. Anything found is the most severe finding in the
report, and if it is already in a pushed commit, say plainly that removal
requires a history rewrite plus rotation of the exposed secret — a
follow-up commit deletes nothing.

## Quality gradient

- `skills/<catalog>/` is the published product: users copy these
  directories out of the repository, where they lose access to everything
  else. Highest scrutiny; installed-behavior defects live only here.
- Everything else (`.agents/`, `.github/`, `scripts/`, `justfile`, READMEs,
  `ARCHITECTURE.md`) is repository harness, not published content. Review
  it for synchronization, not polish: the finding shape is desync — a pair
  in `.agents/knowledge/harness-maintenance.md` changed on one side only,
  a documented command that no longer runs, a pointer to a moved file,
  guidance drifted from the implementation it describes. Stale harness is
  entropy that misleads every later agent; report it, capped at
  recommended severity.
- `openspec/` is the specification layer. A pull request that changes a
  public skill's behavior without a delta spec, or that is ready with a
  Validation section naming no scenario, is blocking. A repository change
  needs its `skip_specs` record instead of a delta spec
  (`spec-workflow.md ## Domains`). A hand edit to a generated `openspec-*`
  skill is a finding (`spec-workflow.md ## Artifacts`).
- Never block a ready pull request for being unarchived: during its second
  deliberation that is the expected state, and `checks / spec` blocks the
  merge until the freeze (`spec-workflow.md ## Archive executor and the
  freeze`).

## Scripts: the threat model

Skill scripts are one-off developer tooling run locally by the coding
agent, on inputs the agent itself supplies. They are not services and have
no untrusted callers — an agent that wanted to act unsafely could act
directly, so a script adds no attack surface. Do not report input
sanitization, injection, or hardening findings against them as security
issues. What does matter:

- Protecting the human from agent mistakes: destructive or remote-writing
  paths default to dry-run and require an explicit `--apply`, `--delete`,
  or `--confirm`.
- Secrets are never printed or logged.
- The script contract in `skill-quality.md ## Scripts` holds.

The rules each kind of script is reviewed against have one owner:

- This repository's code — `scripts/`, workflow shell, the project
  skills' scripts — `.agents/knowledge/management-code.md`; a privileged
  job handles content the request's author wrote, so read its
  `## Workflow security` before reviewing one.
- The Python scripts and script assets public skills ship —
  `skill-quality.md ## Script assets`.

From ryan-minato/skills, Apache-2.0.

More from ryan-minato/skills