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
134 changes: 134 additions & 0 deletions docs/agent-tests/2026-10-03.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# Agent test: do the rules hold without a skill?

Date: 3 Oct 2026.

## Decision

**AGENTS.md is enough, with one new line.** The first three runs showed a gap for CS-8. One line in AGENTS.md closed it in three fresh runs (see "Second round"), so no skill is needed.

First round: a skill would be needed for CS-8.

All three runs break CS-8, "Add no option, hook or abstraction that the spec does not need". Two runs also break CS-5 on a strict reading. One run breaks API-1.

The rule of this test says that two or more results with the same broken rule need a skill. CS-8 meets it.

One gap causes all of this. No rule says what to do when the task needs a symbol that the Interface section does not list. Section "Cheap AGENTS.md change" shows a one-line fix. Try that line first. Re-run the test. Write the skill only if the runs still add the symbol.

## Method

- Task given to each run: "Add a helper that turns a list of user-entered postcodes into canonical forms and reports the bad ones."
- Each run had a clean clone of the js repo on main `c6ad92c`. No run was told about the standards.
- Runs 1, 2 and 3 are the clones `js-1`, `js-2` and `js-3`.
- `pnpm check` ran in copies of the clones. The clones are unchanged.
- Each diff was read against API-1, every TELL rule with the Check value "review", and the review checklist.
- The reason for each choice comes from the commit message and the code. No run was asked.

## Results at a glance

| Run | `pnpm check` | Violations counted | Rules broken |
|---|---|---|---|
| 1 (`canonicalBatch`, internal) | pass | 3 | CS-8, CS-5, glossary |
| 2 (`parseAll`, internal) | pass | 2 | CS-8, CS-5 |
| 3 (`parseAll`, exported) | pass | 2 | API-1, CS-8 |

`pnpm check` passes on all three runs. No tool in CI finds any of these violations. Only review finds them.

## What went well

- All three runs found `AGENTS.md` and the standards without a hint.
- Runs 1 and 2 cite the standard in the commit body. Run 1 cites the TypeScript file. Run 2 cites API-1 by ID.
- All three commits have a sign-off, a scoped Conventional Commits subject under 72 characters and no co-author line.
- No run breaks a TELL rule that needs review: TELL-4, TELL-5, TELL-6, TELL-8, TELL-11, TELL-15, TELL-19 and TELL-20. See "Considered and not counted".
- Each run wrote tests with specific values (T-4) and names in domain words (T-6).
- Each diff touches only the lines that the task needs (TELL-15).
- Run 3 did the whole GIT-7 set for a public change: API report, README, change file with a minor bump, and `@example` (TS-9). It also froze its results, like `parse` does.

## Violations

### Run 1: branch `feat/core-canonical-batch`, commit `c8db361`

1. **CS-8.** `packages/core/src/canonical-batch.ts:41-53` adds `canonicalBatch`, and `:11-30` adds two types. The Interface section of `spec/grammar.md` lists no such function.
2. **CS-5.** The same lines. The function has `@internal`, and `src/index.ts` does not export it. No other module calls it. Only `test/canonical-batch.test.ts` uses it. A caller cannot reach the helper, so the task is not done. TS-9 gives `@internal` to helpers "that other modules use". No module uses this one.
3. **Glossary.** The commit subject uses "canonicalises". `CONTEXT.md` lists "canonicalise" under Avoid for the term Normalise. `AGENTS.md` says to name things with the glossary terms. This is minor. The task text itself says "canonical forms".

Reason given (commit body): the TypeScript standard says that the SDK adds no idiomatic symbols, so the helper stays out of `src/index.ts` and carries `@internal`.

### Run 2: branch `feat/core-parse-all`, commit `8569ba6`

1. **CS-8.** `packages/core/src/parse-all.ts:41-56` adds `parseAll`, and `:11-30` adds two types. The spec does not list them.
2. **CS-5.** The same lines. The function is internal and has no caller outside its test, `test/parse-all.test.ts`.

Reason given (commit body): "It stays internal: API-1 needs a spec change before core exports a new symbol."

Run 2 names the right rule. It then keeps the code. The rule that it cites also says to change the spec first. It did not change the spec and did not stop.

### Run 3: branch `feat/core-parse-all`, commit `6ea05a6`

1. **API-1.** `packages/core/src/index.ts:15` exports `parseAll`. Lines 20 and 26 export the types `ParseAllReport` and `RejectedInput`. `spec/standards/languages/typescript.md`, section "Idiomatic additions (API-1)", says that the TypeScript SDK adds no idiomatic symbols. No spec change came first. The new function is also a new concept, so it needs a spec change, not a language-file change.
2. **CS-8.** `packages/core/src/parse-all.ts:21-39` and `packages/core/src/types.ts:66-86`. The spec does not need these. The new public symbol also puts the JS SDK ahead of the PHP SDK (PAR-1 and PAR-2 are not broken, but the SDKs now differ).

Reason given (commit body): callers that read a list, such as a CSV column, had to loop over `parse` and sort the results by hand. The body cites API-3 and API-4 and calls the function "a thin wrapper over `parse`". The body does not name API-1. The run said outside the diff that it exported the helper despite API-1. That statement is not in the diff, and this report did not check it.

The reason is a good one for a person who wants the feature. It does not replace the process in API-1, which is to change the spec first. API-12 supports the helper. If the helper is deleted, the loop moves into each caller. That makes a case for a spec change. It does not make a case to skip the spec.

## Rule check

| Rule | Run 1 | Run 2 | Run 3 | Runs that break it |
|---|---|---|---|---|
| API-1 | no | no | yes | 1 |
| CS-8 | yes | yes | yes | 3 |
| CS-5 (strict) | yes | yes | no | 2 |

Applying the rule of this test:

- CS-8: 3 runs. A skill is needed.
- CS-5: 2 runs, on a strict reading. Treat it as part of the same fix.
- API-1: 1 run. Alone, it does not need a skill.

## Why a skill, and why it may be avoidable

The agents were not careless. They read the rules, and two of them tried to follow API-1. The standard has a gap in what it tells an agent to do. API-1 says "change the spec first". An agent that works alone on a task cannot change the spec in the same step. So each run chose one of two paths:

- Hide the symbol (runs 1 and 2). This keeps API-1 and breaks CS-8 and CS-5.
- Export the symbol (run 3). This breaks API-1.

The right path was to stop, add nothing, and report that the task needs a spec change first. No run took it. `AGENTS.md` has no line that says this is the right path.

## Cheap AGENTS.md change

Add this item to "Rules that no config file shows" in `AGENTS.md`:

> If the task needs a function, type or constant that the Interface section of `spec/grammar.md` does not list, do not add it, with any visibility. Do not hide it with `@internal`. Stop, and say that a spec change must come first (API-1, CS-8).

This would likely prevent all five violations above, because it covers the three paths that the runs took: export, hide, and keep. It costs two lines.

Add one sentence to the "Done means" list: "A new symbol that no public entry or other module uses is dead code (CS-5)." This closes the `@internal` route.

Two other options, in order of cost:

1. Add the same text to the review checklist in `CODING_STANDARDS.md`, item 1, so that a reviewer checks for a hidden symbol.
2. Add a CI check. A script can compare the exports in `src/index.ts` with the Interface table. It cannot find an `@internal` symbol that has no caller, so use a lint rule for unused exports in that case. This is the strongest fix. It makes the rule a tool rule instead of a review rule.

## Considered and not counted

- **TELL-6, name `result`.** Run 1 uses `result` at `canonical-batch.ts:45`. Run 2 uses it at `parse-all.test.ts:8` and `:13`. The glossary term "Parse result" makes the name meaningful in run 1. Run 2 holds a different type. This is a close call. The tell list names `result`, so a strict reviewer could count two runs. If that happens, add TELL-6 to the skill. The decision stays the same.
- **TELL-5, doc comments such as "The result of `parseAll`".** Runs 1 and 2 have them at `:20-23` in each source file. The repo's own `types.ts` uses the same style ("The result of `parse`: ..."). Not counted.
- **Immutability.** Runs 1 and 2 return plain arrays and objects. `parse` freezes its results. The types are `readonly`, so API-6 and TS-6 hold. Run 3 follows the repo style. Not counted.
- **TELL-8.** No existing helper loops over `parse` in the repo. No duplicate exists.
- **T-1.** Each commit holds the test and the code. The order of work is not visible in the diff. Not judged.
- **Empty change files.** Runs 1 and 2 use an empty change file. `AGENTS.md` allows it when the change needs no release. An internal helper needs none.
- **Run 3 `options` parameter.** `allowPartial` is in the spec for `parse`. It passes through. No new option.

## Second round

Three fresh runs (4, 5 and 6) got the same task, in clean clones with one new line in "Rules that no config file shows" in AGENTS.md:

> If the task needs a public symbol that the Interface section of `spec/grammar.md` does not list, add nothing, not even an internal helper. Stop, and say that a spec change must come first (API-1, CS-8).

| Run | Result |
|---|---|
| 4 | added nothing; said a spec change must come first; listed the open design questions |
| 5 | added nothing; cited API-1 and CS-8; said the owner chooses |
| 6 | added nothing; pointed to `parse` in a loop as the way to do it today |

No run added the symbol. The line goes into AGENTS.md in every repo and into the template.
1 change: 1 addition & 0 deletions templates/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ This repo is part of Gatepost, unofficial open-source developer tools for Nigeri

## Rules that no config file shows

- If the task needs a public symbol that the Interface section of `spec/grammar.md` does not list, add nothing, not even an internal helper. Stop, and say that a spec change must come first (API-1, CS-8).
- Call only the documented NIPOST gateway endpoints.
- Commit only NIPOST's published test codes and synthetic values, such as `FC-01-Z99-ZZ-01`.
- Sign off each commit with `git commit -s`. The person who opens the pull request is the only author. Leave out co-author lines for AI tools.
Expand Down