Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions .agents/skills/agents-md-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
name: agents-md-review
description: Audit an existing AGENTS.md or CLAUDE.md line by line for wrong, stale, discoverable, duplicated, or content-free guidance and propose the pruned file. Use when asked to review, prune, shrink, or clean up one.
---

# AGENTS.md review

Verdicts apply the test in `agents-md`: more than one cheap feedback round to discover and fix, and stated in no skill or always-read file.

## Verify, then classify

Review the file as given and state which revision.
Check each block against reality before judging it; one piece of evidence per verdict is enough.

- Examples and formats against the tool that consumes them (changelog generator, release tooling, CI workflows): **Wrong**.
Reading the tool's configuration is enough; what it would render wrongly is wrong whether or not the bad output has shipped.
- Lists against the tree and the remote default branch: **Stale**.
- Claimed conventions against merged-PR history: **Never a convention** when nobody follows it and the maintainer confirms nobody decided it.
A maintainer may be introducing the rule deliberately, and a human committer proves nothing about who decided it, so an unfollowed rule the maintainer has not ruled on goes on the human-decision list.
- Rules against the installed skills and always-read files: **Duplicated**.
- Everything else against the test above: **Discoverable**, **Content-free**, or **Plain command**.
- An always-loaded pointer to a long guide: **Misplaced**; it belongs in the skill that needs it.

A block can earn more than one verdict, each with its own evidence; lead with the one that needs no human confirmation, so pruning can start before the maintainer answers.

Also look for what is **Missing**: policy the organisation applies elsewhere (intent-file rules, writing conventions, comment markers), non-standard layouts with no pointer, and an `AGENTS.md` with no sibling `CLAUDE.md` symlink.
`agents-md/references/before-after.md` is one repository's worked answer; re-verify every row against the repository in front of you, never paste it.

## Report

```md
| Block | Verdict | Evidence |
|---|---|---|
| `Issue: #456`, `Issue: N/A` as valid footers | Wrong, Never a convention | `cliff.toml` turns every `Issue:` value into a Jira link; maintainer confirms neither was decided |
| Tests drive the CLI binary, not the library; see `testsuite/AGENTS.md` | Keep | redoing a library-level testsuite is more than one round; no skill states it |
```

Then propose the full pruned file, list what moved to a skill or a nested `AGENTS.md`, and list what needs a human decision.
Do not apply the changes unless asked.
53 changes: 53 additions & 0 deletions .agents/skills/agents-md/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
name: agents-md
description: Discipline for writing and updating AGENTS.md (or CLAUDE.md) so it holds only what an agent cannot infer - policy, gotchas, and pointers - and stays under about 40 lines. Use instead of /init when creating, editing, or adapting one.
---

# AGENTS.md

`AGENTS.md` is loaded on every task, so every line is a cost paid everywhere.
Measured on real repositories (arXiv 2602.11988): developer-written files raised resolve rate about 4% and cost about 20%; LLM-generated files lowered resolve rate; repository overviews did not help.
What helped agents was locating the right place to change, not writing better patches.

## The test

A line earns its place when both hold:

> Would it cost more than one cheap feedback round to discover and fix?
> Does no skill or always-read file already state it?

A lint or formatting miss costs one run of CI or of the repository's check command and is not worth a line; a testsuite written against the library instead of the CLI is a pile to redo, and is.
Put positively: there is no way the agent learns this unless we write it.

## What belongs, in order of value

- A pointer to a non-standard layout: "Integration tests are centralized in `testsuite`; see `testsuite/AGENTS.md`."
- The default feedback loop when it is not the ecosystem's default: "`cargo xtask check-uncommitted` verifies local modifications."
- A policy with its reason: "Use `#[expect(lint, reason = "...")]` so the suppression explains itself and expires when it stops being needed."
- Repository rules no tool enforces: intent-file handling, writing conventions for committed text, comment markers such as `INVARIANT:` and `UNTESTED:`.
- The one fact a shared skill leaves to the repository, such as the issue-reference footer format, when it fits in a line; otherwise a repo-local skill.
- One opener saying where skills live, for agents that do not discover them.

What `/init` generates is the blacklist: product overview, directory map, command lists, "follow standard conventions", inventories of packages or scopes, and lossy summaries of `STYLE.md`.
Examples without the rule behind them are the costliest lines; agents copy the example, mistakes included.

## Division of labour

```text
AGENTS.md always loaded policy, gotchas, pointers
skill loaded on match rules for one task; may load the skills it builds on
STYLE.md loaded by a reviewer skill when a rule's intent is unclear
```

The root file enters every context, so about 40 lines is the rule of thumb; a nested file loads for fewer tasks and may run longer; past the mark, flag it for the human rather than cut a gotcha to fit.
Load each file at the latest point where it pays for itself; an always-loaded file pointing at a 700-line guide is a cost on every task, while a skill pointing at the one it depends on keeps both small.
A directory with its own policy gets a nested `AGENTS.md` and one pointer from the root.
Every `AGENTS.md` gets a sibling `CLAUDE.md` symlink to it, because Claude Code reads only `CLAUDE.md`.

## Keeping it true

- Verify every surviving sentence against the tree and the remote default branch; `/init` files go stale within weeks.
- When a line documents a workaround, fix the code instead; the file is a list of smells not yet fixed.
- When adapting another repository's file, report deviations with reasons instead of copying: a line true there may be false here.

`references/before-after.md` holds one 151-line file reduced to 27, with the reason each block was cut or kept and the maintainer's verbatim pushbacks; read it when unsure what counts as discoverable.
65 changes: 65 additions & 0 deletions .agents/skills/agents-md/references/before-after.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Before and after: Devolutions Gateway `AGENTS.md`

`git diff 6b906d7d b6c98d77 -- AGENTS.md` in Devolutions/devolutions-gateway, PR #2091.
151 lines became 27.
IronRDP's equivalent is Devolutions/IronRDP#2131 (155 to 38).
The result is `git show b6c98d77:AGENTS.md`: five headings (intent files, writing for the repository, Rust, untested-property comments, Git) under a one-line skills opener.

## What was cut, and why it was cuttable

| Block in the old file | Verdict | Evidence |
|---|---|---|
| `Issue: #456 (GitHub issue)` as a valid footer | Wrong | `cliff.toml` renders every `Issue:` value as a Jira URL; four forms of the mistake in the last 300 commits |
| Scopes `dgw`, `agent`, `jetsocat`, `webapp`; "library scopes `*deps*`, `*openapi*`, `*npm*`" | Stale | `installer` and `agent-installer` missing; `cliff.toml` suppresses only exact `chore(deps)`/`build(deps)`; the `commit-scope` skill already had the right table |
| Branch naming (`DGW-338-...`, `fix/123-description`) | Never a convention | 2 of the last 60 merged PR branches carried a Jira key |
| ~70 lines of PR title/body/footer/examples | Duplicated | `commit-and-pr-conventions` and `commit-scope` skills |
| ~55 lines of Rust style (errors, logging, comments, invariants, size constants) | Duplicated, lossy | a summary of IronRDP `STYLE.md`, vendored by the same PR and served by the `rust-style` skill |
| `## Project Overview` workspace listing | Discoverable | `Cargo.toml`, `webapp/pnpm-workspace.yaml` |
| `## Additional Resources` (`Main README: README.md`, `Cookbook: docs/COOKBOOK.md`) | Discoverable | one `ls` |
| TypeScript and PowerShell command lists | Discoverable | `webapp/package.json` scripts, `powershell/*.ps1` |
| `Tests go in testsuite/tests/, helpers in testsuite/src/`; `Consider proptest, rstest` | Discoverable | the tree; replaced by one pointer to `testsuite/AGENTS.md` |
| `Follow standard .NET conventions.` / `Follow standard PowerShell conventions.` | Content-free | |
| `cargo clippy --workspace --tests -- -D warnings`, `cargo test --workspace` | Plain command | CI annotations print the command to run |

## What was kept or added, and the test it passed

| Line | Why it earns its place |
|---|---|
| Nightly rustfmt | Kept at the time as a one-round gotcha; under the feedback-round test it is borderline, since CI reports the fix in one round, and the maintainer is inclined to drop it. IronRDP dropped it because it formats with stable: verify before copying |
| `#[expect(..., reason = "...")]` with its reason | Policy; the old text said `THINK before suppressing` without the why |
| Pointer to `testsuite/AGENTS.md` | Non-standard Cargo layout; details live in the nested file (26 lines: run the binary, dependencies are a design signal, fewer stronger tests, timing tests run serially) |
| `Issue: DGW-123` | The shared commit skill defers footer format to the repository; the one-line form stays here and the full rules live in the repo-local `commit-footer` skill the line points to |
| Intent files, writing for the repository, untested-property comments | Policy no agent infers and no skill holds; taken from IronVNC and picky-rs |
| Skills opener | For agents that do not auto-discover `.agents/skills/` |

## Maintainer pushbacks, verbatim

From Gateway, IronVNC, and sspi-rs `AGENTS.md` reviews.

- "it contains redundant information that is better derived by each agents on demand"
- "Remove everything that can be explored naturally (e.g.: layout section)"
- "I would not include any 'obvious' cargo command, it may be worth keeping the line about using nightly for formatting, and to think before silenting lints."
- "I don't think we even need the introduction about what is Devolutions Gateway"
- "I think we can refer to rust-style in the top-level AGENTS.md, but I would keep rust-style-reviewer mentioned only in the code-review skill."
- "Don't we risk to have the agent pull off the full file? Is it better to refer to the STYLE.md file only in the reviewer skill?"
- "Also verify the Git section does not duplicate the other matching skills."
- "The best place for that may be in the matching skill instead of AGENTS.md"
- "The property is that we run them serially, it may be good to explain that" (a timing-tests section had described fixed delays instead of the property)
- "I think this is not necessary, agents already do it without instructions" ([sspi-rs #763](https://github.com/Devolutions/sspi-rs/pull/763#discussion_r4195986262))
- "Since this can be derived easily from the workspace Cargo.toml, I would tend to remove that" ([sspi-rs #763](https://github.com/Devolutions/sspi-rs/pull/763#discussion_r4196024203))
- "Do you think of a specific anti-pattern?" and "I need some more meat on this to understand what that is concretely. What are we asking the agent to do?" ([sspi-rs #763](https://github.com/Devolutions/sspi-rs/pull/763#discussion_r4196018926))
- "I think this is better suited as a review-style skill, rather than the top-level AGENTS.md" ([sspi-rs #763](https://github.com/Devolutions/sspi-rs/pull/763#discussion_r4195993368))
- "It's likely enough to suggest `cargo xtask check wasm`, and remove the note about native threads, filesystem, etc. Even if the agent would use such APIs, it would be blocked by `cargo xtask check wasm`." ([IronVNC #1544](https://github.com/Devolutions/IronVNC/pull/1544#discussion_r4024093374))
- "I would remove the heading of all files where it's not really necessary. This is read by agents, and it's found in a folder called 'ironvnc-fuzzing'." ([IronVNC #1542](https://github.com/Devolutions/IronVNC/pull/1542#discussion_r4013920553))
- The positive form: "Perfect, there is no way the agent can learn about this if we don't write it in the AGENTS.md" ([sspi-rs #763](https://github.com/Devolutions/sspi-rs/pull/763#discussion_r4196027485))

## Resulting reference graph

```text
AGENTS.md ──► rust-style skill (no pointer onward)
code-review skill ──► rust-style-reviewer skill ──► STYLE.md (only when a rule's intent is unclear)
AGENTS.md ──► testsuite/AGENTS.md
AGENTS.md ──► commit-footer skill
```

Each file is loaded at the latest point where it pays for itself.
35 changes: 35 additions & 0 deletions .agents/skills/blind-testsuite/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
name: blind-testsuite
description: Write or extend a conformance suite from a CONTRACT*.md and published sources only, by an author who never sees an implementation, kept read-only while implementations are built against it. Use when a contract has or will have several implementations, or a wrong expected value would be invisible to the implementer, as in cryptography, codecs, and wire protocols.
---

# Blind test suite

The suite is independent evidence derived from the contract, not the definition of conformance, so a failing test says the implementation or the test is wrong, never the contract.
`test-writer` decides each test's shape and `test-oracle-reviewer` what counts as an independent value; this skill is the isolation around them.

## What the author sees

The contract file, the interface it is written against (the contract crate or module, its types and documentation), and published sources: specifications, vendored vectors, captures.
Not an implementation, an earlier implementation, or the pull request that produced the contract, because each carries a reading of the contract the suite would then confirm rather than test.
When an implementation exists, the author is a fresh context in a worktree at a commit without it; a do-not-read list supplements that and does not replace it, since a context that has seen the implementation cannot unsee it.

## Expected values

From a published source cited in the test, a property the contract states, or a derivation whose method the contract or cited standard fixes, recorded beside the vector so a reviewer can redo it; never from a computation only an implementation of the contract could perform, because checking such a value means implementing the contract again.
A derived vector accounts for every coupled field the change touches - length, checksum, signature - or its outcome is not what the derivation claims.
Where no published answer exists, compare outputs across implementations; a differential check never stands alone, because a bug shared by both passes it.
A hand-written negative input differs from an independently valid control in the targeted dimension only, so the test can fail only for the reason it names; a rule that names a rejected input gets one, since such a rule tested only by acceptance is met by accepting everything.
Vendored sources keep origin, version, and checksum recorded, are taken whole or cut at their own section boundaries, and are never edited, because an edited or sliced vector loses the provenance that makes it an oracle.
Keep a coverage record beside the suite: which test checks each clause, which clauses are verified by review only, and the boundaries deliberately left unchecked, so silence is not taken for coverage.

## Review and merge

The reviewer runs `test-reviewer` and checks the contract against the coverage record both ways: every clause has a test or a recorded gap, every test cites its clause.
Write the suite before the implementation; merge it once one implementation runs it green, so a test no conforming implementation can pass is found before it binds every later one.

## Read-only during implementation

An implementation pull request never edits the suite or the contract.
A test that looks wrong, or a clause with no test, is escalated with the vector or clause, expected and observed values, and the cited source.
The fix goes to the pull request that owns the suite - the open one through its lead, or a new one once it has merged - by a fresh author with the same inputs, who receives the decision and not the implementation's output, so the suite stays blind.
1 change: 1 addition & 0 deletions .agents/skills/commit-and-pr-conventions/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,6 @@ Issue: PROJECT-123
- Do not manually wrap the PR body; GitHub handles display wrapping.
- Keep permanent history concise: record motivation, material behavior, and side effects.
- Put experiments, alternatives, extensive validation, and implementation chronology in follow-up comments.
- Post a follow-up comment with the lines changed, split at least into production code, tests, and other files, so a reviewer sees where the change's weight lies before reading it; how to count and present them is yours, and refresh it when the diff changes materially.
- Update the title and initial body as the implementation evolves.
- Before opening, updating, or merging, read `title + blank line + body` as one commit message and correct any mismatch.
Loading
Loading