From 3138c51c61c05db8684fb84fa7ab2ed8dc249309 Mon Sep 17 00:00:00 2001 From: Tiago Quintino Date: Thu, 30 Jul 2026 16:18:49 +0100 Subject: [PATCH 1/4] Move "Agent Skills" to Agents/skills Introduces a top-level `Agents/` area with `skills/` beneath it, so that future agent artefacts (subagents, prompts, evaluation material) have an obvious home alongside skills rather than accumulating in a single skills-only directory. - git mv "Agent Skills" -> Agents/skills (rename detected; no content change) - update all inbound links: root README, Guidelines/Ai-Contributions-To-Software, Legal/Open-Sourcing-Software, ADR-009 - fix outbound relative links inside the moved files, which are now one level deeper (../ -> ../../ in the skills README; ../../ -> ../../../ in both SKILL.md files) Link check: 0 broken. The remaining bare `](LICENSE)` references are inside template blocks showing what a target repository's README should contain, not links within the Codex. --- ADR/ADR-009-Repository-Audit-Store.md | 4 ++-- {Agent Skills => Agents/skills}/README.md | 2 +- .../skills}/open-source-audit/SKILL.md | 8 ++++---- .../skills}/security-audit/SKILL.md | 12 ++++++------ Guidelines/Ai-Contributions-To-Software.md | 2 +- Legal/Open-Sourcing-Software.md | 2 +- README.md | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) rename {Agent Skills => Agents/skills}/README.md (97%) rename {Agent Skills => Agents/skills}/open-source-audit/SKILL.md (98%) rename {Agent Skills => Agents/skills}/security-audit/SKILL.md (98%) diff --git a/ADR/ADR-009-Repository-Audit-Store.md b/ADR/ADR-009-Repository-Audit-Store.md index fda42ac..148097a 100644 --- a/ADR/ADR-009-Repository-Audit-Store.md +++ b/ADR/ADR-009-Repository-Audit-Store.md @@ -145,7 +145,7 @@ automation target. - The report store is the "designated store" referenced by the `open-source-audit` skill in - [`Agent Skills/open-source-audit/SKILL.md`](../Agent%20Skills/open-source-audit/SKILL.md). + [`Agents/skills/open-source-audit/SKILL.md`](../Agents/skills/open-source-audit/SKILL.md). - Reports implement part of the open-sourcing process in [`Legal/Open-Sourcing-Software.md`](../Legal/Open-Sourcing-Software.md). - Supports the **Secure by Design** principle in @@ -180,7 +180,7 @@ This ADR does not modify or supersede any existing ADR. - Report store: (private) and its `SCHEMA.md` - Open-source audit skill: - [`Agent Skills/open-source-audit/SKILL.md`](../Agent%20Skills/open-source-audit/SKILL.md) + [`Agents/skills/open-source-audit/SKILL.md`](../Agents/skills/open-source-audit/SKILL.md) - Open-sourcing process: [`Legal/Open-Sourcing-Software.md`](../Legal/Open-Sourcing-Software.md) diff --git a/Agent Skills/README.md b/Agents/skills/README.md similarity index 97% rename from Agent Skills/README.md rename to Agents/skills/README.md index 88b20a2..bf1218c 100644 --- a/Agent Skills/README.md +++ b/Agents/skills/README.md @@ -2,7 +2,7 @@ Reusable AI-agent skills that support ECMWF software governance processes. These skills are AI agents and operate under the -[AI Contributions to Software](../Guidelines/Ai-Contributions-To-Software.md) +[AI Contributions to Software](../../Guidelines/Ai-Contributions-To-Software.md) guidelines: they keep a human in the loop, and they **report and gate rather than act** — they never merge, fix, or publish on their own. diff --git a/Agent Skills/open-source-audit/SKILL.md b/Agents/skills/open-source-audit/SKILL.md similarity index 98% rename from Agent Skills/open-source-audit/SKILL.md rename to Agents/skills/open-source-audit/SKILL.md index 340c097..55a3583 100644 --- a/Agent Skills/open-source-audit/SKILL.md +++ b/Agents/skills/open-source-audit/SKILL.md @@ -21,7 +21,7 @@ and general open-sourcing best practices **before** it is made public — and (see "Run modes"). This skill is run as part of the ECMWF open-sourcing process described in -[`Legal/Open-Sourcing-Software.md`](../../Legal/Open-Sourcing-Software.md). +[`Legal/Open-Sourcing-Software.md`](../../../Legal/Open-Sourcing-Software.md). It is the final technical gate, run when a repository owner requests the GitHub Enterprise / organisation owner to switch the repository from private to public. The audit is normally run by, or on behalf of, that owner before @@ -32,7 +32,7 @@ and maintainership have already been settled through the official ECMWF procedure. Your job is the technical audit of the repository contents. This file is written to be **model-agnostic** (see the portability notes in -[`Agent Skills/README.md`](../README.md)): the instructions below are +[`Agents/skills/README.md`](../README.md)): the instructions below are self-contained plain Markdown and work whether this skill is loaded by Claude, GPT or Gemini agents. @@ -206,7 +206,7 @@ Decide which mode applies before you start, and state it in the report. Apache, etc.) may be included only if its original copyright and licence notice are preserved *and* it is recorded in the `NOTICE` file alongside `LICENSE` (see - [Copyright-And-Licensing.md](../../Legal/Copyright-And-Licensing.md)). + [Copyright-And-Licensing.md](../../../Legal/Copyright-And-Licensing.md)). Stripped attribution, or a missing/incomplete `NOTICE` for included third-party code, is a FAIL. If the provenance or licence of a block cannot be established, treat it as an IPR risk: FAIL, and refer to the @@ -243,7 +243,7 @@ Decide which mode applies before you start, and state it in the report. the GitHub–Zenodo integration) and add a `CITATION.cff` file at the repository root so downstream users and publications can cite it precisely. A copy-ready example is in the Codex at - [`Repository Structure/CITATION.cff`](../../Repository%20Structure/CITATION.cff). + [`Repository Structure/CITATION.cff`](../../../Repository%20Structure/CITATION.cff). This is an **advisory**, never a blocker (it is typically completed once the repository is public — see "Post-publication recommendations"). diff --git a/Agent Skills/security-audit/SKILL.md b/Agents/skills/security-audit/SKILL.md similarity index 98% rename from Agent Skills/security-audit/SKILL.md rename to Agents/skills/security-audit/SKILL.md index 17fd86d..c2a1300 100644 --- a/Agent Skills/security-audit/SKILL.md +++ b/Agents/skills/security-audit/SKILL.md @@ -24,14 +24,14 @@ licensing, README/maturity, secrets-in-history and basic supply-chain hygiene; **this** skill is the dedicated, deeper security review. This skill is run as part of the ECMWF open-sourcing process -([`Legal/Open-Sourcing-Software.md`](../../Legal/Open-Sourcing-Software.md)), +([`Legal/Open-Sourcing-Software.md`](../../../Legal/Open-Sourcing-Software.md)), by a GitHub Enterprise / organisation owner. Its report is filed in the private audit store **`ecmwf/repo-audits`** as a `Security-Audit` (see that repository's `SCHEMA.md`). It supports the **Secure by Design** principle -([`Principles/Open-Source-Principles.md`](../../Principles/Open-Source-Principles.md)). +([`Principles/Open-Source-Principles.md`](../../../Principles/Open-Source-Principles.md)). This file is written to be **model-agnostic** (see the portability notes in -[`Agent Skills/README.md`](../README.md)): the instructions below are +[`Agents/skills/README.md`](../README.md)): the instructions below are self-contained plain Markdown and work whether this skill is loaded by Claude, GPT or Gemini agents. @@ -190,7 +190,7 @@ and private-key markers in addition to running `gitleaks`. If the audited repository is **already public** and you confirm a CRITICAL or HIGH vulnerability, additionally recommend that the owner tracks the fix as a **draft security advisory** (private fork, coordinated release) per the -[Security Vulnerability Disclosure](../../Guidelines/Security-Vulnerability-Disclosure.md) +[Security Vulnerability Disclosure](../../../Guidelines/Security-Vulnerability-Disclosure.md) procedure — do not describe the vulnerability in any public issue or PR. You still only report and recommend; opening the advisory is the owner's action. @@ -355,10 +355,10 @@ Check settings with `gh api` where you have access, otherwise note as Unverified vulnerability-disclosure policy. ECMWF's reporting route is **GitHub private vulnerability reporting (PVR) first**, with the Support Portal () as fallback, per the - [Security Vulnerability Disclosure](../../Guidelines/Security-Vulnerability-Disclosure.md) + [Security Vulnerability Disclosure](../../../Guidelines/Security-Vulnerability-Disclosure.md) procedure; recommend adding a `SECURITY.md` that points reporters there. A ready-to-use, generic template is in the Codex at - [`Repository Structure/SECURITY.md`](../../Repository%20Structure/SECURITY.md). + [`Repository Structure/SECURITY.md`](../../../Repository%20Structure/SECURITY.md). Its absence is a **LOW** recommendation, never a FAIL. - [ ] **Private vulnerability reporting enabled** *(advisory, non-blocking during rollout)* — public ECMWF repositories must have PVR enabled per the diff --git a/Guidelines/Ai-Contributions-To-Software.md b/Guidelines/Ai-Contributions-To-Software.md index ab2023d..7340435 100644 --- a/Guidelines/Ai-Contributions-To-Software.md +++ b/Guidelines/Ai-Contributions-To-Software.md @@ -19,7 +19,7 @@ At ECMWF, we view AI Code Agents and Large Language Models (LLMs) as powerful pr ### Code Review Protocols * **Mandatory Review:** Code generated by AI Agents is always subject to review, as customary for any code contribution. This should include both automated checks and human peer review. This code review may itself be assisted by AI agents, but the final review and approval must be performed by a human reviewer. -* **Reviews by AI Agents:** If you are using an AI agent to support code reviews, a good practice is to have a different AI agent model review the code, to provide a form of "cross-checking" and reduce the risk of blind spots or biases in a single model. ECMWF ships reusable governance agents for this purpose — the [Agent Skills](../Agent%20Skills/README.md) (e.g. `open-source-audit`, `security-audit`), which report and gate but never merge or publish on their own. +* **Reviews by AI Agents:** If you are using an AI agent to support code reviews, a good practice is to have a different AI agent model review the code, to provide a form of "cross-checking" and reduce the risk of blind spots or biases in a single model. ECMWF ships reusable governance agents for this purpose — the [Agent Skills](../Agents/skills/README.md) (e.g. `open-source-audit`, `security-audit`), which report and gate but never merge or publish on their own. * **Peer Reviewing:** AI-generated code should be reviewed by a human *other* than the developer who prompted or controlled the AI agent. We recognise this may not always be possible, but it should be the preferred approach. * **Operational Note:** This independent peer-review approach becomes even more important, preferred and highly recommended for any code entering operational, critical, or heavily relied-upon execution paths. diff --git a/Legal/Open-Sourcing-Software.md b/Legal/Open-Sourcing-Software.md index 9eb67f0..263f629 100644 --- a/Legal/Open-Sourcing-Software.md +++ b/Legal/Open-Sourcing-Software.md @@ -41,7 +41,7 @@ When open sourcing an ECMWF software, either as an ECMWF member or as a sub-cont - IPR violations. Contact Development Section to arrange for a code audit - Sensitive information that is internal to ECMWF (e.g. passwords, user names, hostnames, emails, etc.) - Ensure that the code is not using any third party software that is not compatible with the Apache Licence - - Run the [open-source-audit agent skill](../Agent%20Skills/open-source-audit/SKILL.md) + - Run the [open-source-audit agent skill](../Agents/skills/open-source-audit/SKILL.md) over the repository as the final technical gate before it is made public. This audit is triggered when a repository owner asks the GitHub Enterprise / organisation owner (the contact points above) to switch the diff --git a/README.md b/README.md index a809373..3f0ea6a 100644 --- a/README.md +++ b/README.md @@ -22,4 +22,4 @@ The Codex is a set of principles and guidelines for development of software and - [Documentation and Training](./Documentation%20and%20Training) - [Contributing to External Projects](./Contributing%20Upstream) - [Software Management Plan](./Software%20Management%20Plan) -- [Agent Skills](./Agent%20Skills) +- [Agent Skills](./Agents/skills) From 2c27a468d817607114eb5ce75f3332e5d4614d97 Mon Sep 17 00:00:00 2001 From: Tiago Quintino Date: Thu, 30 Jul 2026 16:22:08 +0100 Subject: [PATCH 2/4] Agents/skills: rewrite README as skill-authoring guidance The previous README predated a cross-vendor standard and stated that none existed. The Agent Skills specification (https://agentskills.io/specification) now fills that gap, so this rewrites the README around it. Adds: - what a skill is, and the progressive-disclosure model agents use - directory layout (SKILL.md, scripts/, references/, assets/) - the full frontmatter table with the specification's constraints, including the name regex and the rule that name must match the directory - guidance on writing the body: explicit decision rules, stop conditions, and restating the trigger outside the frontmatter - size budgets per load stage, and the 500-line SKILL.md ceiling - validation via skills-ref - ECMWF house rules layered on the specification: report-and-gate, model-agnostic, never invent or hide findings, redact secrets, British English, project-agnostic - installation paths per harness, preferring the vendor-neutral .agents/skills/ location, plus a table of per-product setup documentation from https://agentskills.io/clients - a checklist for contributing a new skill --- Agents/skills/README.md | 289 ++++++++++++++++++++++++++++++++++------ 1 file changed, 247 insertions(+), 42 deletions(-) diff --git a/Agents/skills/README.md b/Agents/skills/README.md index bf1218c..b061293 100644 --- a/Agents/skills/README.md +++ b/Agents/skills/README.md @@ -1,12 +1,27 @@ # Agent Skills -Reusable AI-agent skills that support ECMWF software governance processes. These -skills are AI agents and operate under the +Reusable AI-agent skills that support ECMWF software governance processes. + +These skills are AI agents and operate under the [AI Contributions to Software](../../Guidelines/Ai-Contributions-To-Software.md) guidelines: they keep a human in the loop, and they **report and gate rather than act** — they never merge, fix, or publish on their own. -## Skills +## Contents + +- [Available skills](#available-skills) +- [What a skill is](#what-a-skill-is) +- [Authoring a skill](#authoring-a-skill) + - [Directory layout](#directory-layout) + - [Frontmatter](#frontmatter) + - [Writing the body](#writing-the-body) + - [Progressive disclosure and size budgets](#progressive-disclosure-and-size-budgets) + - [Validation](#validation) +- [ECMWF house rules](#ecmwf-house-rules) +- [Installing a skill](#installing-a-skill) +- [Contributing a skill](#contributing-a-skill) + +## Available skills - [`open-source-audit`](./open-source-audit/SKILL.md) — technical open-source compliance audit of a repository. Run before it is made public, and re-run any @@ -20,43 +35,233 @@ than act** — they never merge, fix, or publish on their own. findings — see the `security-audit` skill) is a blocker. - [`security-audit`](./security-audit/SKILL.md) — risk-tiered security audit of a repository, run before publication (the security step of the open-source - audit) and periodically afterwards. Builds a threat model, - runs SAST / dependency / supply-chain tooling, reviews security-sensitive - surfaces, and for high-risk repositories adds adversarial testing and bounded - fuzzing. Produces a CWE-tagged pass/fail report. - -## Format and portability - -Skills here follow the [Agent Skills](https://www.anthropic.com/news/skills) -convention: a self-contained folder with a `SKILL.md` file whose YAML -frontmatter carries a `name` and a `description` of when to use the skill, -followed by plain-Markdown instructions. - -There is no cross-vendor standard for agent skills yet, so we author them to be -**model-agnostic** — the same `SKILL.md` should work with Claude, OpenAI GPT and -Google Gemini agents. To keep a skill maximally portable: - -- **Treat the frontmatter as metadata, not logic.** Keep it to `name` and - `description`. A harness that does not parse frontmatter simply sees it as - leading text, which is harmless. Never place an instruction that appears - *only* in the frontmatter — restate the trigger in the first paragraph of the - body so nothing is lost if the frontmatter is ignored or stripped. -- **Keep the body self-contained plain Markdown.** It must read correctly on its - own, using imperative prose and standard Markdown — no provider-specific - markup (no XML-style tags, no vendor-only directives). -- **Reference tools generically.** Prefer portable shell commands (`gitleaks`, - `git log`, `gh api`) over "use the *X* tool", so the skill does not depend on + audit) and periodically afterwards. Builds a threat model, runs SAST / + dependency / supply-chain tooling, reviews security-sensitive surfaces, and + for high-risk repositories adds adversarial testing and bounded fuzzing. + Produces a CWE-tagged pass/fail report. + +## What a skill is + +A skill is a self-contained folder containing a `SKILL.md` file: YAML +frontmatter carrying a `name` and a `description`, followed by plain-Markdown +instructions. Agents load skills *progressively* — the description is read at +startup so the agent knows the skill exists, and the body is loaded only when +the skill is actually needed. + +Skills in this directory follow the +**[Agent Skills specification](https://agentskills.io/specification)**, an +open, cross-vendor format supported by a wide range of agent products. Writing +to the specification is what makes a skill portable: the same folder works in +Claude Code, OpenCode, GitHub Copilot, Cursor, Gemini CLI and others without +modification. + +## Authoring a skill + +### Directory layout + +At minimum a skill is a directory with a `SKILL.md`. Everything else is +optional: + +``` +skill-name/ +├── SKILL.md # required: frontmatter + instructions +├── scripts/ # optional: executable helpers the agent can run +├── references/ # optional: detailed docs, loaded on demand +└── assets/ # optional: templates, schemas, lookup data +``` + +- **`scripts/`** — self-contained executables, with documented dependencies, + helpful error messages, and graceful handling of edge cases. +- **`references/`** — detail that would bloat `SKILL.md`. Keep each file + focused; the agent loads them only when required, so smaller files cost less + context. +- **`assets/`** — static resources such as report templates or schemas. + +### Frontmatter + +| Field | Required | Constraints | +| --------------- | -------- | ----------- | +| `name` | Yes | 1–64 characters. Lowercase letters, digits and single hyphens only. Must not start or end with a hyphen, must not contain `--`, and **must match the parent directory name**. | +| `description` | Yes | 1–1024 characters. Describes **what** the skill does and **when** to use it. | +| `license` | No | Licence name, or a reference to a bundled licence file. | +| `compatibility` | No | Max 500 characters. Environment requirements — required tools, network access, intended product. Most skills do not need this. | +| `metadata` | No | Arbitrary string-to-string map. Use reasonably unique keys to avoid collisions. | +| `allowed-tools` | No | Space-separated pre-approved tools. **Experimental** — support varies between agents; prefer describing constraints in prose. | + +The regex a valid `name` must satisfy is `^[a-z0-9]+(-[a-z0-9]+)*$`. + +The `description` is the single most important field: it is what an agent sees +when deciding whether the skill is relevant. State the trigger explicitly and +include the keywords a user would actually type. + +```yaml +# Good — says what it does and when to reach for it +description: >- + Audits a repository for open-source publication readiness: licensing, + per-file headers, secret scanning across full git history, and CI + configuration. Use before making a repository public, or to re-check an + already-public repository. + +# Poor — an agent cannot tell when this applies +description: Helps with audits. +``` + +### Writing the body + +The Markdown after the frontmatter is the instruction set. There are no format +restrictions, but the following work well: + +- **Imperative, step-by-step instructions.** Say what to do, in order. +- **Explicit decision rules.** State what is a blocker versus advisory, what + severity applies, and what the agent must never do. Ambiguity is where agents + drift. +- **Worked examples** of inputs and outputs, and the common edge cases. +- **Stop conditions.** For any loop, state precisely what terminates it. +- **Restate the trigger in the first paragraph.** If a harness ignores or + strips frontmatter, an instruction that existed only there is lost. + +Reference other files by relative path from the skill root, and keep references +one level deep — avoid chains of files that each point to another: + +```markdown +See [the reporting format](references/REPORT-FORMAT.md) for the full schema. +Run `scripts/collect-evidence.sh` to gather the raw findings. +``` + +### Progressive disclosure and size budgets + +Agents load a skill in three stages, so structure it accordingly: + +| Stage | What loads | Budget | +| ----- | ---------- | ------ | +| 1. Metadata | `name` + `description`, for **every** installed skill, at startup | ~100 tokens | +| 2. Instructions | the whole `SKILL.md` body, once the skill is activated | < 5000 tokens recommended | +| 3. Resources | files under `scripts/`, `references/`, `assets/`, on demand | as needed | + +Keep `SKILL.md` **under 500 lines**. If it grows past that, move detailed +reference material into `references/`. Stage 1 is charged against every session +whether or not the skill is used, so a bloated description has a cost across the +board. + +### Validation + +Validate a skill against the specification with the +[`skills-ref`](https://github.com/agentskills/agentskills/tree/main/skills-ref) +reference library: + +```bash +skills-ref validate ./skill-name +``` + +This checks the frontmatter and the naming rules. Run it before opening a pull +request. + +## ECMWF house rules + +On top of the specification, skills in this directory must: + +- **Report and gate, never act.** A governance skill produces a verdict and + evidence. It does not merge pull requests, publish artefacts, rotate + credentials, or change repository visibility. Those remain human decisions, + per [AI Contributions to Software](../../Guidelines/Ai-Contributions-To-Software.md). +- **Be model-agnostic.** The same `SKILL.md` should work with any competent + agent. No provider-specific markup, no XML-style tags, no vendor-only + directives. +- **Reference tools generically.** Prefer portable commands (`git log`, + `gh api`, `gitleaks`) over "use the *X* tool", so a skill does not depend on one agent's tool names or calling convention. -- **Avoid provider-specific frontmatter keys** (for example `allowed-tools`). If - a capability or constraint matters, describe it in prose instead. - -How to supply a skill to each agent: - -- **Claude / Claude Code / Claude Agent SDK** — place the skill folder in the - agent's skills directory; the frontmatter is discovered automatically and the - body is loaded on demand. -- **OpenAI GPT (Assistants, custom GPTs, Codex CLI)** — provide `SKILL.md` as a - developer/system message, or attach it as context/a file. The body works - as-is. -- **Google Gemini (Gems, API system instructions)** — provide `SKILL.md` as a - system instruction or attached file. +- **Never invent findings, and never hide them.** Distinguish what was checked + and passed from what could not be checked. If something could not be + verified, say so explicitly rather than reporting it as a pass. +- **Keep secrets out of output.** Redact credential values in any report; never + reproduce a live secret, even one already committed. +- **Use British English** in prose, consistent with the rest of the Codex. + (Field names from the specification — `license`, `allowed-tools` — keep their + original spelling.) +- **Be project-agnostic.** A skill here is meant to be reused across ECMWF + repositories and adapted outside them. Discover the repository's own build, + test and documentation layout rather than hard-coding paths, tool names or + organisation names. + +## Installing a skill + +A skill is installed by placing its folder where the agent looks for skills. +The layout inside the folder never changes — only the parent directory differs +between products, and several products additionally read each other's +locations. + +**Common locations** + +| Scope | Path | Read by | +| ----- | ---- | ------- | +| Vendor-neutral, project | `.agents/skills//SKILL.md` | OpenCode, and other agent-compatible clients | +| Vendor-neutral, global | `~/.agents/skills//SKILL.md` | as above | +| Claude-compatible, project | `.claude/skills//SKILL.md` | Claude Code, OpenCode | +| Claude-compatible, global | `~/.claude/skills//SKILL.md` | Claude Code, OpenCode | +| OpenCode, project | `.opencode/skills//SKILL.md` | OpenCode | +| OpenCode, global | `~/.config/opencode/skills//SKILL.md` | OpenCode | + +Prefer the **vendor-neutral `.agents/skills/`** path where the harness supports +it, and commit project-scoped skills to the repository so the whole team gets +them. + +For example, to install the `open-source-audit` skill into a repository for any +agent-compatible client: + +```bash +mkdir -p .agents/skills +cp -r /path/to/codex/Agents/skills/open-source-audit .agents/skills/ +``` + +**Per-product setup instructions** + +Support and exact paths are defined by each product. Consult the official +documentation: + +| Product | Setup instructions | +| ------- | ------------------ | +| Claude Code | | +| Claude | | +| OpenCode | | +| GitHub Copilot | | +| VS Code | | +| Cursor | | +| Gemini CLI | | +| OpenAI Codex | | +| JetBrains Junie | | +| Goose | | +| Amp | | +| Factory | | +| Roo Code | | +| OpenHands | | +| Kiro | | + +The full, current list of products supporting the format is maintained at +. + +Some harnesses gate skill loading behind permissions. OpenCode, for example, +controls this through `opencode.json`: + +```json +{ + "permission": { + "skill": { "*": "allow" } + } +} +``` + +If a skill does not appear: check that the file is named `SKILL.md` in capitals, +that the frontmatter has both `name` and `description`, that `name` matches the +directory name, and that the name is unique across all searched locations. + +## Contributing a skill + +1. Create `Agents/skills//SKILL.md`, with `name` matching the directory. +2. Write it to the specification and to the house rules above — project-agnostic, + language-agnostic, report-and-gate. +3. Run `skills-ref validate ./Agents/skills/` and keep `SKILL.md` under + 500 lines. +4. Add the skill to [Available skills](#available-skills) in this README. +5. Open a pull request. Skills that gate a governance process (publication, + disclosure, release) should say so explicitly, and should reference the Codex + document they enforce. From 0f162a1d3c90b16438aed6df89f2a084d89ea796 Mon Sep 17 00:00:00 2001 From: Tiago Quintino Date: Thu, 30 Jul 2026 16:41:02 +0100 Subject: [PATCH 3/4] Agents/skills: add five reusable development skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ports five workflows into project- and language-agnostic skills written to the Agent Skills specification. Each discovers the repository's own build, test and documentation layout rather than hard-coding paths, tools or organisation names. - review-pull-request — works through review feedback until it converges. Merges two prior workflows (respond-to-review and the Copilot review loop), since the mechanics are identical and only the reviewer differs. Records the hard-won bot mechanics: the literal `[bot]` reviewer suffix is the only request form that works, and the `requested_reviewers` list cycling back to empty is the terminating signal. Notes that `gh pr view` does not show inline comments, so the GraphQL reviewThreads query is required. Never merges. - doc-fact-check — executes runnable doc examples and verifies falsifiable claims against source. Keeps the distinction between a documentation bug and an environment problem, and refuses to invent imports to make a block run. Proposes fixes without applying them. - onboard-repository — read-only survey producing a structured mental model. Trusts code over documents and reports the drift between them. - robustness-audit — merges error-handling and edge-case auditing, which belong together: an unhandled edge case usually surfaces as a bad error. Severity ladder led by silent-wrong-answer. Asks before changing ambiguous or public behaviour. - improve-code-coverage — treats coverage as a proxy rather than a goal. Categorises each gap (testable / defensive / dead / environment-specific), prioritises by risk over line count, and deletes dead code rather than suppressing it. Also splits the README index into governance skills (which gate and never act) and development skills (which may change code, but still never merge, publish or release, and ask before changing public behaviour). All five validate against the specification: name matches directory, name regex and length, description present and within 1024 characters, no unknown frontmatter keys, and each SKILL.md under the 500-line ceiling. --- Agents/skills/README.md | 49 +++- Agents/skills/doc-fact-check/SKILL.md | 167 ++++++++++++ Agents/skills/improve-code-coverage/SKILL.md | 160 ++++++++++++ Agents/skills/onboard-repository/SKILL.md | 145 +++++++++++ Agents/skills/review-pull-request/SKILL.md | 253 +++++++++++++++++++ Agents/skills/robustness-audit/SKILL.md | 192 ++++++++++++++ 6 files changed, 959 insertions(+), 7 deletions(-) create mode 100644 Agents/skills/doc-fact-check/SKILL.md create mode 100644 Agents/skills/improve-code-coverage/SKILL.md create mode 100644 Agents/skills/onboard-repository/SKILL.md create mode 100644 Agents/skills/review-pull-request/SKILL.md create mode 100644 Agents/skills/robustness-audit/SKILL.md diff --git a/Agents/skills/README.md b/Agents/skills/README.md index b061293..727ac2f 100644 --- a/Agents/skills/README.md +++ b/Agents/skills/README.md @@ -10,6 +10,8 @@ than act** — they never merge, fix, or publish on their own. ## Contents - [Available skills](#available-skills) + - [Governance skills](#governance-skills) + - [Development skills](#development-skills) - [What a skill is](#what-a-skill-is) - [Authoring a skill](#authoring-a-skill) - [Directory layout](#directory-layout) @@ -23,6 +25,15 @@ than act** — they never merge, fix, or publish on their own. ## Available skills +Skills here fall into two kinds, which differ in how much authority they have. +**Governance skills** gate a process and therefore never act: they produce a +verdict and evidence for a human to act on. **Development skills** assist with +ordinary engineering work and may change code — but they still stop short of +merging, publishing or releasing, and they ask before changing behaviour that is +ambiguous or public. + +### Governance skills + - [`open-source-audit`](./open-source-audit/SKILL.md) — technical open-source compliance audit of a repository. Run before it is made public, and re-run any time to confirm an already-public repository still complies. Checks repository @@ -40,6 +51,37 @@ than act** — they never merge, fix, or publish on their own. for high-risk repositories adds adversarial testing and bounded fuzzing. Produces a CWE-tagged pass/fail report. +### Development skills + +- [`review-pull-request`](./review-pull-request/SKILL.md) — works through review + feedback on a pull request until it converges. Fetches every unresolved review + thread (including the inline comments that `gh pr view` does not show), + triages each as a fix or a reasoned rejection, applies focused commits, + replies to and resolves each thread, and drives an automated reviewer such as + the GitHub Copilot bot around repeated rounds until it stops producing new + findings. It never merges the pull request. +- [`doc-fact-check`](./doc-fact-check/SKILL.md) — verifies documentation against + the code it describes. Executes the runnable examples and checks falsifiable + claims (API names and signatures, defaults, enum values, counts, CLI flags, + configuration keys) against the source, reporting each mismatch as an error, a + stale claim or a drift. Proposes fixes without applying them. +- [`onboard-repository`](./onboard-repository/SKILL.md) — builds a working + mental model of an unfamiliar repository: reads its intent documents, maps its + structure and build system, surveys a representative slice of the code, and + identifies the conventions and the gate that constrain any change. Read-only; + reports drift between the documentation and the code. +- [`robustness-audit`](./robustness-audit/SKILL.md) — audits how code behaves + when things go wrong: error paths (crashes on library paths, swallowed causes, + missing diagnostic context, inconsistent mapping across boundaries) and edge + cases (empty and boundary inputs, `NaN` and infinities, index limits, + concurrency, filesystem and encoding hazards). Fixes findings with tests, and + asks before changing ambiguous or public behaviour. +- [`improve-code-coverage`](./improve-code-coverage/SKILL.md) — measures + coverage, categorises every gap as untested-but-testable, defensive, dead or + environment-specific, then closes the real gaps with tests that assert + behaviour, prioritising error paths and risky code. Deletes genuinely dead + code rather than suppressing it, and reports before and after. + ## What a skill is A skill is a self-contained folder containing a `SKILL.md` file: YAML @@ -228,13 +270,6 @@ documentation: | Cursor | | | Gemini CLI | | | OpenAI Codex | | -| JetBrains Junie | | -| Goose | | -| Amp | | -| Factory | | -| Roo Code | | -| OpenHands | | -| Kiro | | The full, current list of products supporting the format is maintained at . diff --git a/Agents/skills/doc-fact-check/SKILL.md b/Agents/skills/doc-fact-check/SKILL.md new file mode 100644 index 0000000..a71ab66 --- /dev/null +++ b/Agents/skills/doc-fact-check/SKILL.md @@ -0,0 +1,167 @@ +--- +name: doc-fact-check +description: >- + Verifies documentation against the code it describes: executes the runnable + code examples in the docs and checks falsifiable prose claims (API names, + signatures, defaults, enum values, counts, CLI flags, configuration keys) + against the actual source. Reports each mismatch as an error, a stale claim or + a drift, and proposes fixes without applying them. Use when asked to + fact-check documentation, find documentation drift, or verify that docs and + code still agree. +license: Apache-2.0 +--- + +# Fact-check documentation against the code + +Use this skill when you are asked to verify that a project's documentation still +matches its code — to run the examples in the docs, to find stale or drifted +claims, or to check the docs before a release. + +The governing rule: **the code is the truth**. Where documentation and code +disagree, the documentation is wrong until a human says otherwise. Report the +disagreement; do not "fix" the code to match the docs. + +This skill **proposes fixes but does not apply them**. Documentation wording is +an editorial decision. Present findings for review unless the user explicitly +asks you to apply them. + +## Inputs + +The user may narrow the scope to particular files or directories. With no scope +given, check all prose documentation in the repository. + +## 1. Locate the documentation and the source + +Discover both rather than assuming a layout. Look for, in no particular order: + +- a documentation directory (`docs/`, `doc/`, `documentation/`, `site/`, + `website/`, `book/`, `manual/`); +- a documentation generator's configuration, which names the source directory + (Sphinx `conf.py`, MkDocs `mkdocs.yml`, mdBook `book.toml`, Docusaurus, + Jekyll `_config.yml`, Doxygen `Doxyfile`, Quarto `_quarto.yml`); +- top-level prose: `README`, `CONTRIBUTING`, `CHANGELOG`, and any guide or + tutorial files; +- documentation embedded in source as docstrings or API doc comments, if the + user's scope includes it. + +Identify the languages in the repository from its build manifests, so you know +which examples you can execute and where to look for the definitions a claim +refers to. + +## 2. Prepare an environment + +Work out how the project is built and made importable, from its own contributing +documentation and build files. Build or install what the examples need. + +If a setup step fails, **that is not a documentation bug**. Report it separately +as an environment problem, then continue with whatever parts of the project do +work. Never report a block as failing when the real cause is a missing tool or +an unbuilt dependency. + +## 3. Classify every fenced code block + +For each fenced block in the selected documents, decide one of three: + +- **Runnable** — self-contained: it has whatever imports, includes or entry + point its language needs, and contains no elided placeholders (`...`, + ``, `TODO`). +- **Continuation** — it depends on state established by an earlier runnable + block on the same page. Concatenate it with its predecessors and run the + combined program. +- **Skip** — anything else. Skip: partial fragments, signature-only listings, + sample *output* rather than input, blocks with no language tag, installation + and setup commands, and anything requiring user-specific data, credentials, + network access to a private service, or hardware the environment lacks. + +Record the classification for every block, including the skips and why. + +## 4. Execute the runnable blocks + +Run each runnable (or concatenated) block in an isolated temporary location, +with a per-block timeout — around 30 seconds is a reasonable default. Capture +the exit status and the output. + +General approach by language family: + +- **Interpreted languages** (Python, JavaScript, Ruby, R, shell): write the + block to a temporary file and execute it with the project's own interpreter + or environment. +- **Compiled languages** (Rust, C, C++, Go, Java, Fortran): only blocks that + form a complete compilable unit. Create a scratch project that depends on the + library **by path within the repository**, so the local build is what gets + tested rather than a published release. Build it, run it if it produces a + binary, then clean up. +- **Shell examples**: only those that can run without user-specific data and + without mutating the user's system. Check for a zero exit status. Never run a + command that writes outside the temporary area, deletes data, publishes, or + contacts a production service. + +Two absolute rules: + +- **Never invent missing imports, variables or arguments** to make a block run. + If it is not self-contained, it is a skip — and if the documentation presents + it as complete, that itself is a finding. +- **Never modify the documentation to make a block pass.** Record the failure. + +## 5. Check falsifiable claims against the source + +Read the prose and extract claims that can be mechanically checked. In practice +these appear as inline code spans, numbers, and table cells — not as +paragraphs of explanation. + +**Check:** + +| Claim type | How to verify | +| ---------- | ------------- | +| API names and signatures — types, functions, methods, fields, parameters, return types | Find the definition in the source and compare, using symbol navigation where available | +| Enum members, constants, magic values, error codes | Compare against the actual definitions | +| Default values ("defaults to 60 seconds", "row-major by default") | Read the default in the code — the constructor, the default implementation, the argument declaration | +| Counts ("supports 12 formats", "over 200 tests") | Count them | +| CLI commands, subcommands and flags | Run the tool's `--help` and compare | +| Configuration and environment variable names | Find where they are read | +| Feature or build flags | Compare against the build manifests | +| Supported versions ("requires Python 3.9+") | Compare against the declared metadata | +| Cross-references and links | Resolve them; a link to a moved or deleted file is a finding | + +**Do not check** subjective or architectural statements: "fast", "efficient", +"designed for scale", explanations of *why* something works a certain way. +These are not falsifiable and flagging them produces noise. + +When a claim is ambiguous — you cannot tell what would make it true or false — +leave it alone and say so rather than inventing an interpretation. + +## 6. Report + +Report each finding in a consistent form, with the file and line, what the +documentation asserts, and what the source actually shows: + +``` +[ERROR] guide/api.md:42 — documented parameter does not exist + Docs say: connect(host, port, timeout=30) + Code says: connect(host, port, timeout_seconds=30.0) src/client.py:88 +``` + +Classify each finding: + +- **ERROR** — a code example fails, or a documented name, signature or command + does not exist. The documentation is actively misleading. +- **STALE** — the documented thing existed once but has been renamed, removed or + replaced. +- **DRIFT** — a small mismatch: a count that is slightly off, a default that + changed, a version bound that has moved. + +Close with a summary: + +- files checked; +- blocks classified runnable / continuation / skipped, and how many ran, passed + and failed; +- claims checked, and findings by category; +- environment problems, listed separately from documentation findings; +- anything you could not verify, and why. + +For each ERROR, propose a concrete fix — the exact replacement text — but do not +apply it. Present the set for review. + +Be explicit about coverage. "All examples pass" must mean every runnable block +was executed, not that none were attempted. If most blocks were skipped, say so +prominently: a green report over an unexecuted corpus is worse than no report. diff --git a/Agents/skills/improve-code-coverage/SKILL.md b/Agents/skills/improve-code-coverage/SKILL.md new file mode 100644 index 0000000..1cf3fa8 --- /dev/null +++ b/Agents/skills/improve-code-coverage/SKILL.md @@ -0,0 +1,160 @@ +--- +name: improve-code-coverage +description: >- + Measures test coverage, categorises every gap as untested-but-testable, + defensive, dead, or environment-specific, then closes the real gaps with tests + that assert behaviour — prioritising error paths and risky code over easy + lines. Removes genuinely dead code rather than suppressing it, and reports a + before-and-after comparison. Use when asked to improve or measure test + coverage, or to find and fill gaps in a test suite. +license: Apache-2.0 +--- + +# Improve test coverage + +Use this skill when you are asked to measure test coverage, raise it, or find +what a test suite is not exercising. + +The governing rule: **coverage is a proxy, not the goal.** The goal is a suite +that catches regressions. A line executed by a test that asserts nothing is +worse than an uncovered line, because it reports safety that does not exist. +Never write a test whose purpose is to move a number. + +## Inputs + +The user may give a target percentage and a scope. If no target is given, adopt +the project's own configured threshold if it has one; otherwise ask rather than +assuming a number. Treat any target as a direction of travel, not a quota to be +met by any means. + +## 1. Find the coverage tooling + +Discover how this project measures coverage rather than assuming a tool. Check, +in order: + +- the CI workflows — if coverage is measured anywhere, it is usually there, with + the exact invocation and any threshold already configured; +- the build and test configuration files, and any coverage configuration + (exclusion lists, thresholds, report formats); +- the contributing documentation. + +If the project has no coverage tooling, select the standard instrument for its +language and say clearly in the report that you introduced it, along with the +invocation you used — a baseline measured with an unfamiliar tool is not +comparable to anything the project has seen before. + +Note any existing exclusions, and treat them as deliberate until shown +otherwise. + +## 2. Measure the baseline + +Run the full suite under coverage and record, per file: lines covered, lines +total, and the specific uncovered line ranges. Keep this baseline — the final +report compares against it. + +If parts of the suite cannot run in this environment, record which and why. Their +absence will depress coverage in ways that are not real gaps, and reporting that +as a finding would be wrong. + +## 3. Categorise every gap + +This is the step that determines whether the work is useful. For each uncovered +region, read the code and assign one of four categories. Do not skip to writing +tests. + +**(a) Untested but testable.** Reachable behaviour with no test. These are the +real gaps and the main target. + +**(b) Defensive.** Guards against conditions that are hard to trigger — an +invariant violation, an impossible branch, an allocation failure. Some can be +reached with dependency injection, fault injection or a fake; those move to (a). +Those that genuinely cannot be reached without contorting the design should stay +uncovered and be excluded explicitly, with a reason. + +**(c) Dead.** Genuinely unreachable: superseded branches, unused helpers, +conditions that cannot hold. The fix is deletion, not a test. + +**(d) Environment-specific.** Reachable only on another platform, +architecture, or optional feature configuration. Note whether CI covers it in a +different job; if so, it is covered, just not here. If nothing covers it, that is +a real finding worth reporting even if you cannot close it locally. + +## 4. Prioritise by risk, not by line count + +Rank the category (a) gaps. Chasing the largest uncovered file first is the +common mistake; the largest gap is often generated code or a simple data module. + +Prioritise: + +1. **Error and failure paths** — the least-tested and most-likely-to-be-wrong + code in most projects. +2. **Input parsing and validation**, especially of untrusted or external data. +3. **Boundaries** — foreign function interfaces, language bindings, serialisation, + anything crossing a process or network edge, where failures are silent and + expensive. +4. **Security- and correctness-critical logic** — authentication, authorisation, + permissions, numerical kernels whose results are trusted downstream. +5. **Recently changed or historically buggy code**, which the version history + will tell you. +6. Everything else. + +## 5. Write tests that assert behaviour + +- Add to the **existing** test files and follow the project's established + patterns, fixtures and naming. A suite with two competing styles is worse than + one with a gap. +- Each test should target a specific uncovered path and **assert an observable + outcome** — a returned value, a raised error and its message, a recorded side + effect. Calling a function and checking it did not crash is not a test. +- Test error paths as first-class cases: assert the *specific* failure, not + merely that something failed. +- Use adversarial inputs for validation code — malformed, empty, boundary and + oversized. +- Keep tests deterministic. No dependence on wall-clock time, network access, + execution order, or unseeded randomness. +- Prefer a few tests that capture the real contract over many that restate the + implementation. Tests that mirror the code line by line break on every + refactor and catch nothing. + +If writing a test reveals that the code is untestable without changing its +design, say so rather than forcing an awkward test. That is a finding. + +If a new test fails because the **code** is wrong, stop and report it. You have +found a bug, which is a better outcome than the coverage change; do not quietly +adjust the test to match the broken behaviour. + +## 6. Handle dead code and exclusions + +For each (c): verify it is genuinely unreachable — search for all callers, +including dynamic dispatch, reflection, generated bindings, configuration-driven +paths, and use from other languages in the repository. Public interfaces may be +used by consumers outside the repository entirely; those are not dead, and +removing them is a breaking change requiring a human decision. + +Once verified: **delete it.** Do not mark it with a coverage-suppression +comment. Suppression hides the code from measurement while leaving it to rot. + +Use exclusions only for category (b) and (d), only per-region rather than +per-file, and always with a stated reason. Never widen an exclusion to make a +threshold pass — that is the coverage equivalent of deleting a failing test. + +## 7. Verify and report + +Run the full suite to confirm everything passes, then re-measure coverage the +same way as the baseline. + +Report: + +- **Before and after**, per file, plus the overall figure — measured + identically, or the comparison is meaningless. +- **Tests added**, and which gap each closes. +- **Code deleted**, with the evidence that it was unreachable. +- **Gaps deliberately left**, by category, with the reason: defensive, + environment-specific, or requiring a design change. +- **Bugs found** while writing tests — usually the most valuable output. +- **Anything not measured**, such as suites that could not run here. + +State plainly whether the target was met, and if it was not, what remains and +why. A short honest report that says "84%, and the remaining 16% is defensive +and platform-specific code, itemised below" is worth more than reaching a +threshold by excluding what was inconvenient. diff --git a/Agents/skills/onboard-repository/SKILL.md b/Agents/skills/onboard-repository/SKILL.md new file mode 100644 index 0000000..7a4cd52 --- /dev/null +++ b/Agents/skills/onboard-repository/SKILL.md @@ -0,0 +1,145 @@ +--- +name: onboard-repository +description: >- + Builds a working mental model of an unfamiliar repository by reading its + intent documents, mapping its structure and build system, surveying a + representative slice of its code, and identifying the conventions and the + build/test/lint gate that constrain any change. Produces a structured + onboarding summary and changes nothing. Use when starting work on an + unfamiliar codebase, or when asked to onboard, orient, or explain how a + repository works. +license: Apache-2.0 +--- + +# Onboard to a repository + +Use this skill when you are starting work on a repository you do not know, or +when asked to explain how one is put together. The goal is a shared mental model +accurate enough to act competently — not a summary of the documentation. + +Three rules: + +- **Derive everything from the repository as it exists right now.** Do not rely + on recollection of the project, on a similarly named project, or on how such + projects usually work. If you did not read it in this repository, do not + assert it. +- **Change nothing.** This is a read-only survey. Do not edit files, do not run + builds that write into the tree beyond ordinary build outputs, and do not run + anything destructive. +- **Where documents and code disagree, the code wins** — and the disagreement is + itself one of the most valuable things to report. + +## Inputs + +The user may give a focus area (a subsystem, a language, an interface). If +given, go deeper there during the code survey while still doing the full +documentation and structure pass. With no focus, spread attention evenly. + +## 1. Read the intent documents + +These say what the project is *for* and what rules apply. Discover them rather +than assuming names — glob the repository root and any documentation directory, +and read what is actually there. + +Look for: + +- the root prose: `README`, `CONTRIBUTING`, `CODE_OF_CONDUCT`, `SECURITY`, + `LICENSE`, `NOTICE`, `CHANGELOG`; +- agent or assistant instructions (`AGENTS.md`, `CLAUDE.md`, `.cursorrules`, + `.github/copilot-instructions.md`) — these often state the project's + non-negotiable rules most directly, and frequently one is a symlink to + another; +- architecture and decision records: an `ADR/` or `decisions/` directory, + `docs/design/`, `plans/`, `rfcs/`, or design documents anywhere in the docs + tree; +- the documentation site source, if there is one. + +From the changelog, read only the unreleased section and the most recent few +releases — enough to know the current direction. Do not read the whole history. + +Distinguish **committed direction** from **speculation**. A document that +describes itself as ideas, brainstorming or future work is not a statement of +how the code behaves today. Treat it accordingly and say so in the report. + +## 2. Map the structure and the build + +Establish the layout without reading every file. + +Identify the build system from its manifests, and let that tell you the +component layout — for example a workspace or monorepo manifest listing members, +a package manifest, a project or module file, or a top-level build script that +orchestrates several of these. Multi-language repositories often have one +orchestrating entry point (a `Makefile`, a task runner, a CI workflow) that +calls into per-language builds; find it and **read its target graph without +running it**. + +Establish: + +- the components or packages, and which language each is written in; +- how they depend on one another, and which is the core; +- where tests, examples, benchmarks and documentation live; +- how the pieces are published or deployed, if that is in scope; +- what the CI workflows actually run — this is often the most reliable + statement of the project's real quality gate. + +## 3. Survey a representative slice of the code + +Read enough source to understand *how the project does what it does*. This is +deliberately not exhaustive. Aim for: + +- **The public surface** of the core component: its primary types, its entry + points, what a consumer is expected to call. +- **The central operation, traced end to end.** Pick the thing the project + exists to do and follow it from the entry point to the result. This single + trace is usually worth more than reading many files in isolation. +- **One integration or binding**, if the project has several — whichever is most + relevant to the user's focus area, otherwise the richest one. This reveals the + pattern the others follow. +- **The error model**: how failures are represented, propagated and surfaced at + the boundaries. +- **Project-wide invariants** the code enforces — determinism, ordering, + compatibility guarantees, thread-safety, resource ownership. Cross-check what + the design documents claim against what the code actually does. + +Prefer symbol navigation over full-text search where it is available; fall back +to searching and reading. Read tests when the intended behaviour of something is +unclear — tests state expectations more precisely than prose usually does. + +## 4. Find the gate + +Determine the canonical commands to build, test, format and lint — the check +that must pass before a change is considered done. Prefer a single aggregate +command if the project defines one, and note whether CI runs something broader. + +Record any heavier checks that exist but are deliberately not part of the normal +gate (extended test matrices, mutation testing, fuzzing, performance runs), and +note that they are opt-in. + +Do not run the gate as part of onboarding unless the user asks. Report what it +is. + +## 5. Report + +Produce a concise, skimmable summary. Aim for something a competent engineer +could read in a few minutes and then act on. + +1. **Purpose** — what the project is and the problem it solves, in two or three + lines. +2. **Architecture** — the components and how they relate. A short list or a + small diagram; not a file listing. +3. **Core data flow** — the central operation traced end to end. +4. **Conventions that constrain edits** — the rules a contributor must respect: + commit and branching conventions, the review and merge process, versioning + and release rules, error and panic policy, compatibility guarantees, anything + with a single source of truth that must not be edited by hand. Cite where + each rule comes from. +5. **How to build, test and lint** — the canonical gate. +6. **Current direction and open work** — what is active versus speculative. +7. **Drift and contradictions** — where documentation disagrees with the code, + where two documents disagree, or where a stated rule is not actually enforced. +8. **What you did not cover** — the areas left unsurveyed, so the reader knows + the boundaries of the model. + +Keep it tight. The output is a mental model, not a copy of the documentation. If +the repository is large, it is better to be accurate about a well-chosen slice +and honest about the rest than to be vague about everything. diff --git a/Agents/skills/review-pull-request/SKILL.md b/Agents/skills/review-pull-request/SKILL.md new file mode 100644 index 0000000..da1b251 --- /dev/null +++ b/Agents/skills/review-pull-request/SKILL.md @@ -0,0 +1,253 @@ +--- +name: review-pull-request +description: >- + Works through review feedback on a pull request until it converges: fetches + every unresolved review thread, triages each comment as fix or reasoned + rejection, applies focused commits, replies to and resolves each thread, and + drives an automated reviewer (such as the GitHub Copilot review bot) around + repeated rounds until it stops producing new findings. Use when asked to + address PR comments, respond to a code review, or run a review loop on a pull + request. +license: Apache-2.0 +--- + +# Review a pull request + +Use this skill when you are asked to address review comments on a pull request, +respond to reviewer feedback, or run an automated review loop until it +converges. It covers both human reviewers and review bots; the mechanics of +fetching, replying to and resolving threads are the same for each. + +Two rules govern everything below: + +- **The loop is the contract.** One round is not convergence. Keep going until a + stated stop condition holds. +- **You never merge.** This skill responds to review; a human decides when the + pull request lands. Do not merge, do not force-push, and do not push at all if + the repository's conventions require approval first. + +## Inputs + +The user supplies a pull request reference. Accept any of: a number (`64`), a +full URL, or `owner/repo#number`. If only a number is given, resolve the +repository from the current checkout. If nothing is given, find the open pull +request for the current branch. Confirm the resolved target before acting. + +Throughout, ``, `` and `` stand for the resolved values. + +## 1. Establish the current state + +Get the pull request's state, mergeability, and check status before changing +anything: + +```bash +gh pr view --repo / \ + --json title,state,isDraft,mergeable,mergeStateStatus,reviewDecision,headRefName +gh pr checks --repo / +``` + +Note the base branch and whether the branch is behind — a review comment may +already be obsolete because the base moved. + +## 2. Fetch every piece of feedback + +**Inline review comments do not appear in `gh pr view`.** Use the GraphQL +`reviewThreads` query, which is also the only way to see which threads are +already resolved: + +```bash +gh api graphql -f query=' +query($owner:String!, $repo:String!, $pr:Int!) { + repository(owner:$owner, name:$repo) { + pullRequest(number:$pr) { + reviewThreads(first:100) { + nodes { + id + isResolved + isOutdated + path + comments(first:20) { + nodes { databaseId author { login } body path line originalLine } + } + } + } + } + } +}' -f owner= -f repo= -F pr= +``` + +Threads where `isResolved` is `true` are done — do not reopen them. Threads +where `isOutdated` is `true` refer to code that has since changed; re-read the +current code before deciding whether the point still stands. + +Also collect the review bodies and any top-level discussion, which carry +findings that are not attached to a line: + +```bash +gh api repos///pulls//reviews +gh api repos///issues//comments +``` + +Build a single list of open items. Deduplicate: reviewers frequently raise the +same point inline and in the review summary. + +## 3. Triage each comment + +For every open item, decide one of three outcomes. Record the reason. + +**Fix** — the finding is correct. Make the change. + +**Reject** — the finding does not warrant a change. Legitimate grounds are: + +- it is factually wrong about what the code does; +- it misreads a deliberate design decision that is documented somewhere; +- it proposes a change that would degrade correctness, clarity or performance; +- it contradicts the project's stated conventions; +- it is out of scope for this pull request and belongs in its own issue. + +A rejection is never silent. It gets a reply that states the reason in one or +two sentences, and points at the evidence (a file and line, a design document, a +prior decision). + +**Ask** — the intent is genuinely ambiguous, or resolving it would change public +behaviour, alter an interface, or reverse a decision you cannot verify. Put the +question to the user rather than guessing. + +Never accept a suggestion just because a reviewer made it. An automated reviewer +in particular will produce confident false positives; applying them uncritically +makes the code worse. Equally, never dismiss a finding merely because fixing it +is inconvenient. + +## 4. Apply the fixes + +- Follow the repository's own commit conventions. Discover them from + `CONTRIBUTING.md`, an agent-instructions file, or the existing `git log` + style — do not impose a convention the project does not use. +- Make **focused commits**: one concern per commit. Keep renames, moves and + reformatting in commits separate from behavioural changes, so a reviewer can + read the diff. +- Update tests and documentation alongside the code when a fix changes + behaviour or a public interface. +- Run the repository's own gate (its build, test and lint commands) before + pushing. Discover it rather than assuming; see the project's contributing + documentation or build files. + +## 5. Reply and resolve + +Every open thread gets a reply, whether it was fixed or rejected. Reply to the +thread rather than posting a new top-level comment, using the `databaseId` of +the comment being answered: + +```bash +gh api repos///pulls//comments \ + -f body='Fixed in : .' \ + -F in_reply_to= +``` + +Then resolve the thread with its node `id` from the query in step 2: + +```bash +gh api graphql -f query=' +mutation($threadId:ID!) { + resolveReviewThread(input:{threadId:$threadId}) { + thread { isResolved } + } +}' -f threadId= +``` + +Resolve a thread only once it is genuinely addressed — either the fix is pushed, +or the rejection reason is posted. Do not resolve threads to make a counter go +down. + +## 6. Push and verify + +Push the fix commits, then wait for the checks: + +```bash +gh pr checks --repo / --watch +``` + +If a check fails, fix it and push again. A red gate is not convergence. If a +check fails for reasons unrelated to the change (known infrastructure +flakiness), say so explicitly in the report rather than quietly ignoring it. + +Push the fixes **before** requesting another review round, so the reviewer sees +the current state and not the state you have already moved past. + +## 7. Requesting a review from an automated reviewer + +For the GitHub Copilot review bot, the literal-`[bot]`-suffix form of the REST +API is the only request that works: + +```bash +gh api --method POST \ + /repos///pulls//requested_reviewers \ + -f 'reviewers[]=copilot-pull-request-reviewer[bot]' +``` + +The alternatives fail, in ways that are easy to misread as success: + +| Attempt | Result | +| ------- | ------ | +| `copilot-pull-request-reviewer` (no suffix) | `422 Reviews may only be requested from collaborators` | +| `Copilot` | returns `200` but the request is silently dropped | +| GraphQL `requestReviews` mutation | `NOT_FOUND` for the bot's node id | + +A successful request responds with +`requested_reviewers: [{login: "Copilot", type: "Bot"}]`, and the review is +posted within a few minutes. + +Other review bots differ. If the product is not Copilot, check its own +documentation for how a review is requested, and record what worked. + +## 8. The loop and when to stop + +Repeat: request a review → wait → triage → fix → reply and resolve → push → +request again. + +While waiting, poll **both** endpoints together: + +```bash +gh api repos///pulls//reviews # has the review count risen? +gh api repos///pulls//requested_reviewers # is the bot still queued? +``` + +After a successful request, `requested_reviewers` lists the bot; it clears back +to `[]` when the bot has finished processing. That gives two outcomes: + +- **A new review is posted** (the review count rises) — go back to triage. +- **`requested_reviewers` cleared and no new review appeared** — the reviewer + processed the request and had nothing to add. **Stop.** The cleared list is + itself the terminating signal; do not run a confirmation round. + +Allow roughly five minutes for a bot to process a request. If the list has not +cleared by then, request again. + +**Stop the loop** when any of these holds: + +- the reviewer processed a request without posting a new review (the cycle + `[bot] → []` completed and the review count did not rise); +- the latest review contains no new actionable findings — every comment is a + duplicate of one already resolved, or a false positive already rejected with a + stated reason; +- all threads are resolved and the checks are green, with no review pending. + +The loop terminates on the reviewer's behaviour, not on a fixed number of +rounds. A single round meeting a stop condition is enough. + +**Escalate to the user instead of looping** if the same finding recurs after you +have fixed it twice, if the reviewer contradicts itself between rounds, or if a +fix would require a decision from step 3's "ask" category. + +## 9. Report + +Finish with a summary that maps every item to its outcome: + +- **Fixed** — the finding, and the commit that addresses it. +- **Rejected** — the finding, and the one-line reason given to the reviewer. +- **Asked** — anything referred back to the user, still open. +- **State** — threads resolved versus outstanding, check status, and whether the + loop terminated or was escalated. + +State plainly whether the pull request is ready for a human decision. Do not +merge it. diff --git a/Agents/skills/robustness-audit/SKILL.md b/Agents/skills/robustness-audit/SKILL.md new file mode 100644 index 0000000..d9a5170 --- /dev/null +++ b/Agents/skills/robustness-audit/SKILL.md @@ -0,0 +1,192 @@ +--- +name: robustness-audit +description: >- + Audits how code behaves when things go wrong: error paths (crashes in library + code, swallowed causes, missing diagnostic context, inconsistent mapping + across language or API boundaries) and edge cases (empty and boundary inputs, + NaN and infinities, index limits, concurrency, filesystem and encoding + hazards). Classifies each finding by severity, fixes them with tests, and asks + before changing behaviour that is ambiguous or public. Use when asked to audit + error handling, harden edge cases, or improve robustness. +license: Apache-2.0 +--- + +# Audit robustness: error paths and edge cases + +Use this skill when you are asked to audit or improve how code behaves under +failure and at its limits — error handling, edge cases, hardening, or general +robustness. + +The two halves belong together: an unhandled edge case usually surfaces as a bad +error, and a bad error usually hides an unhandled edge case. Audit them in one +pass. + +Governing rules: + +- **Fail loudly, never silently.** A wrong answer returned confidently is worse + than an error. The worst outcome this audit can find is code that swallows a + problem and continues. +- **Ask before changing intended semantics.** If it is unclear what *should* + happen for an input — is an empty input an error or a valid no-op? — that is a + design question. Put it to the user. Do not encode a guess as behaviour. +- **Do not change public behaviour silently.** Tightening validation on a public + interface can break consumers. Flag it, and let a human decide. + +## Inputs + +The user may scope the audit to a module, a component or a language. With no +scope, cover the repository's own code, excluding vendored third-party sources. + +## 1. Establish what the project already promises + +Before judging anything, find the project's stated policy. Read its contributing +and design documentation for rules on error handling, on whether library code +may terminate the process, on compatibility guarantees, and on how errors cross +its public boundaries. Read a few existing error paths to learn the established +pattern. + +An audit that imposes a foreign convention is noise. Judge the code against its +own stated rules first, and only then against the general principles below — +flagging where the project has no stated rule as its own finding. + +## 2. Audit the error paths + +Work through these categories. They are language-neutral; the specific construct +differs, the question does not. + +**Crashes on the library path.** Library code should return errors to its caller +rather than terminate the process. Look for the language's abrupt-exit +constructs used outside tests, examples and entry points: unchecked unwrapping +of optional or fallible values, assertions used for input validation, explicit +abort or exit calls, uncaught arithmetic or bounds failures, unchecked casts. +Distinguish a genuine invariant violation (a bug, where terminating may be +correct) from a reachable input condition (which must be an error). + +**Swallowed failures.** The highest-severity pattern in this audit. Look for: + +- empty catch or ignore blocks; +- catching broadly and continuing as if nothing happened; +- discarding a return value that reports failure; +- error translation that drops the original cause, so the chain is lost; +- logging an error and then proceeding on the failure path anyway. + +**Missing diagnostic context.** An error must let someone diagnose the problem +without a debugger. It should identify what failed and on what — the path, the +index, the key, the offending value, the expected versus actual. "Invalid input" +is a finding. + +**Boundary mapping.** Wherever an error crosses a boundary — a foreign function +interface, a language binding, a network or process boundary, a public API — +check that it maps consistently: that every failure is representable on the far +side, that no failure can propagate as undefined behaviour across an interface +that cannot carry it, that message and category survive the crossing, and that +the same underlying failure looks the same from every binding. + +**Documented failure modes.** Public operations should document what they can +fail with, and the documentation should match reality. + +## 3. Audit the edge cases + +Enumerate candidate inputs and states systematically. Not all categories apply +to all code; state which you considered and which were not applicable. + +**Data and values** + +- Empty: zero-length collections, empty strings, empty files, absent optional + fields, empty configuration. +- Single-element and single-dimension cases, and scalar/zero-dimensional cases + where the code otherwise assumes a shape. +- Numeric boundaries: minimum and maximum of each integer type, overflow and + underflow on arithmetic, division by zero, precision loss on conversion. +- Floating point specifically: `NaN` (including its non-reflexive comparison), + positive and negative infinity, negative zero, subnormals, and any comparison + or sort that assumes a total order. This matters especially for numerical and + scientific code, where such values arrive in real data rather than only in + tests. +- Very large inputs: allocation limits, integer overflow in size computations, + quadratic behaviour that only shows at scale. + +**Interfaces and state** + +- Indices and ranges: negative, zero, one past the end, inverted ranges. +- Absent or null arguments where a value is expected. +- Calls made out of order, or on an object already closed, consumed or moved. +- Concurrency: shared mutable state, re-entrancy, iteration during mutation, + and any documented thread-safety claim — test the claim. +- Resource exhaustion: file descriptors, memory, connection pools; and whether + resources are released on the *error* path, not just the success path. + +**External world** + +- Filesystem: missing paths, permission denied, read-only targets, a full disk, + symlinks and path traversal, path separators and case sensitivity across + platforms, paths with spaces or non-ASCII characters, files changing during a + read. +- Partial and interrupted input: truncated data, short reads, cancellation + midway, timeouts. +- Malformed input: corrupt headers, wrong magic bytes, garbage between records, + deeply nested or pathologically large structures. Where such input is + *untrusted*, this overlaps with security — hand those findings to a security + audit rather than treating them as mere robustness. + +**Text** + +- Non-ASCII throughout, including in identifiers, keys and paths; multi-byte + characters split across buffer boundaries; normalisation; very long strings; + embedded null bytes; encoding mismatches. + +**Across boundaries** + +- Where the same logic exists in more than one language or implementation, + check the edge cases agree. Divergence at the edges is a common and + long-lived class of bug. + +## 4. Classify every finding + +| Severity | Meaning | +| -------- | ------- | +| **Critical** | Silent wrong results, data corruption, or undefined behaviour. | +| **High** | Crash or process termination reachable from valid input on a library path. | +| **Medium** | Failure is reported, but the cause is lost or the message is not diagnosable. | +| **Low** | Correct behaviour, poor ergonomics: vague wording, missing documentation. | + +Record for each: location, the category above, the triggering condition, the +observed behaviour, and the behaviour you believe is correct — plus whether that +is stated by the project or inferred by you. + +## 5. Fix, with tests + +Work in order of severity. For each fix: + +- Add a test that **fails before the fix and passes after**. An edge-case fix + without a test will regress. +- Put tests in the existing test files, matching the project's structure and + style. +- Make the failure explicit and diagnosable rather than merely avoiding the + crash. +- Keep fixes focused — one concern per commit. +- Do not suppress a warning or disable a check to make a finding disappear. + +Where step 3 raised a question about intended semantics, or where a fix would +change public behaviour, **stop and ask**. Present the options and their +consequences. + +Consider property-based or fuzz testing for input-parsing code, where it +generates edge cases far more thoroughly than hand-written examples. + +## 6. Verify and report + +Run the project's full gate to confirm no regressions, and re-run the new tests +to confirm they exercise what they claim. + +Report: + +- findings by severity, each with location, trigger and resolution; +- fixes applied, with the test that covers each; +- **questions outstanding** — the ambiguous semantics and public-behaviour + changes awaiting a decision, listed prominently rather than buried; +- categories considered and found not applicable, so the reader knows the scope; +- anything you could not exercise, and why. + +Be honest about coverage. This audit's value is in what it examined; a clean +report over a narrow slice, presented as a clean bill of health, is misleading. From 6fd98e96068237f8130741a221767d70fffaed3c Mon Sep 17 00:00:00 2001 From: Tiago Quintino Date: Thu, 30 Jul 2026 21:37:44 +0100 Subject: [PATCH 4/4] review-pull-request: handle a reviewer that returns a service error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by running the skill against this very pull request: Copilot replied with 'unable to review ... reached their quota limit'. That is posted as a review, so the review count rises while no finding is produced. An agent following the previous wording would treat the rising count as findings and loop forever: every retry posts another error review, so the count keeps rising and the no-new-review stop condition is never reached. Adds a third polling outcome for a service-error review, with the rule to read the review body before treating a rising count as findings, a cap of one retry, and an explicit instruction to report that the pull request has NOT been reviewed — so an unreviewed pull request is never mistaken for a clean one. --- Agents/skills/review-pull-request/SKILL.md | 26 +++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/Agents/skills/review-pull-request/SKILL.md b/Agents/skills/review-pull-request/SKILL.md index da1b251..b91ea28 100644 --- a/Agents/skills/review-pull-request/SKILL.md +++ b/Agents/skills/review-pull-request/SKILL.md @@ -213,12 +213,30 @@ gh api repos///pulls//requested_reviewers # is the bot still qu ``` After a successful request, `requested_reviewers` lists the bot; it clears back -to `[]` when the bot has finished processing. That gives two outcomes: +to `[]` when the bot has finished processing. That gives three outcomes: -- **A new review is posted** (the review count rises) — go back to triage. +- **A new review is posted with findings** (the review count rises) — go back to + triage. - **`requested_reviewers` cleared and no new review appeared** — the reviewer processed the request and had nothing to add. **Stop.** The cleared list is itself the terminating signal; do not run a confirmation round. +- **A review is posted, but it is a service error rather than findings** — for + example a quota or rate limit reached, the service being unavailable, or the + diff being too large to review. **Read the review body before treating a + rising review count as findings.** + +That third outcome needs care, because it can produce an endless loop: each +request returns a "review", so the count rises every round and the +no-new-review stop condition is never reached, while no finding is ever +produced. When the body reports a service error rather than a finding: + +- retry **at most once** — some errors are transient; +- if the second attempt returns the same error, **stop and tell the user**, + naming the error and how many attempts were made; +- do not treat the error review as a finding, do not reply to it as though it + were, and do not resolve anything on account of it; +- state plainly in the report that the pull request **has not been reviewed**, + so that an unreviewed pull request is never mistaken for a clean one. Allow roughly five minutes for a bot to process a request. If the list has not cleared by then, request again. @@ -230,7 +248,9 @@ cleared by then, request again. - the latest review contains no new actionable findings — every comment is a duplicate of one already resolved, or a false positive already rejected with a stated reason; -- all threads are resolved and the checks are green, with no review pending. +- all threads are resolved and the checks are green, with no review pending; +- the reviewer returned the same service error twice (see above) — the loop + cannot make progress, and the pull request remains unreviewed. The loop terminates on the reviewer's behaviour, not on a fixed number of rounds. A single round meeting a stop condition is enough.