Skip to content

Add AGENTS.md, and a CLAUDE.md pointer to it - #27

Merged
mattpodwysocki merged 2 commits into
mainfrom
agents-md
Sep 18, 2026
Merged

mattpodwysocki merged 2 commits into
mainfrom
agents-md

Conversation

@mattpodwysocki

Copy link
Copy Markdown
Contributor

This repo already assumes the file exists

rust-toolchain.toml ends its explanation with:

# Bumping it is its own PR — see "Conventions" in AGENTS.md for what that PR
# has to show.

There's no AGENTS.md to see, so a reader who follows that pointer learns a requirement exists but not what it is — worse than not mentioning it. That's the only dangling reference; the other AGENTS.md matches in the tree are the files generate-skills writes.

A map, not a manual

Every module here already opens with a //! block explaining what it's for and what it refuses to do. Those stay the authority — a second description of the code is a second thing to keep in step, and it would rot within a release. So this file says which one to read, which invariants are held by something other than a reviewer's memory, and which mistakes this repo has already made once.

What it covers:

  • What's generated, and what only looks itopenapi/ is derived, so an edit survives until the next regeneration and no longer; docs/commands.md is hand-written, and docs_contract catches a renamed command but not a parameter description that quietly stopped being true
  • The output contract and the three separate things that enforce it, since a println! is such an easy thing to add
  • The single HTTP client, and why a client built elsewhere fails quietly rather than loudly
  • Values that reach a URLpath_segment, and update_check's shape restriction, both framed as fixes rather than precautions
  • The guards, and that adding yourself to one of their allowlists is almost never the fix
  • What cargo test cannot tell you — it has never sent a request Mapbox read
  • Conventions, compatibility, and the changelog rules

Plus the two habits that have actually caught things here: verify the claim rather than the diff (revert the fix and watch the test fail), and say what you didn't check.

Every claim was checked against the code

Not remembered — I got one wrong on the first pass and caught it. The MAY_WRITE_STDOUT entries are four modules, not four commands, and telemetry.rs is in there because it reads stdout().is_terminal() rather than because it writes. Also verified: the Payload variants, output::emit/output::progress, path_segment's four characters, no_module_builds_its_own_client, the CONTRIBUTING.md anchor it links, and that build.rs reads PINNED_SOURCE.

The new file is covered by the guard from day one

AGENTS.md goes into prose_files, so prose_is_american_english holds the document that states the conventions to them. Verified by planting a British spelling:

AGENTS.md:39: honour -> honor    → FAILED

Note on ordering

Stacked on #25, because it cites that guard by name and adds the file to its list. The diff shrinks to just AGENTS.md, CLAUDE.md and one line of source_guards.rs once #25 lands. 597 tests, fmt and clippy clean.

@mattpodwysocki
mattpodwysocki requested a review from a team as a code owner September 15, 2026 19:54
@zmofei

zmofei commented Sep 16, 2026

Copy link
Copy Markdown
Member

Nice writeup. A few things I ran into locally that this doesn't mention yet — take or leave, just leaving them as reference:

  • Windows-gated code can't be checked from a Mac. cargo check --target x86_64-pc-windows-msvc fails here: ring's build script needs assert.h, which isn't on macOS (verified against this repo just now). To check #[cfg(windows)] code anyway, lift it into a standalone file with stub consts and run rustc/clippy-driver directly against it, no Cargo, no ring.
  • Testing the installer scripts without installing pwsh system-wide — download a PowerShell release tarball into a scratch dir and run ./pwsh/pwsh from there.
  • actionlint before pushing a workflow change — valid YAML isn't a valid workflow (shell: in particular takes no expression context), and a bad workflow file fails with no job and no log.
  • mapbox --schema for exploring the command surface instead of running --help on each command in turn.

Happy to open a follow-up PR for these if useful, otherwise feel free to fold in whatever's worth keeping.

This repository already assumes the file exists. `rust-toolchain.toml` ends
its explanation with "see 'Conventions' in AGENTS.md for what that PR has to
show", and there is no AGENTS.md to see — a reader who follows it learns that
a requirement exists but not what it is. That is the only dangling reference;
the other AGENTS.md matches in the tree are the files `generate-skills`
writes.

It is a map, not a manual. Every module here already opens with a `//!` block
explaining what it is for and what it refuses to do, and those stay the
authority — a second description of the code is a second thing to keep in
step, and it would rot. So the file says which one to read, which invariants
are held by something other than a reviewer's memory, and which mistakes this
repository has already made once.

What it covers: what is generated versus what only looks generated (`openapi/`
is derived and an edit to it survives until the next regeneration;
`docs/commands.md` is hand-written and is the half that drifts), the output
contract and the three separate things that enforce it, the single HTTP
client, the helpers for values that reach a URL, the guards in
tests/source_guards.rs and why adding yourself to one of their lists is
almost never the fix, what `cargo test` cannot tell you, the conventions, and
the compatibility and changelog rules.

Every specific claim in it was checked against the code rather than
remembered: the four modules in MAY_WRITE_STDOUT and why each is there, the
`Payload` variants, `output::emit` and `output::progress`, `path_segment`'s
four characters, `no_module_builds_its_own_client`, the anchor it links in
CONTRIBUTING.md, and that `build.rs` reads PINNED_SOURCE.

AGENTS.md is added to `prose_files`, so `prose_is_american_english` covers it
from the start — the document stating the conventions is held to them.
Verified by planting a British spelling and watching it fail at AGENTS.md:39.

Stacked on the American English branch, since it cites that guard by name.
597 tests, fmt and clippy clean.

@zmofei zmofei left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One more small thing this doc could mention.

MAPBOX_CLI_VERSION needs a v in front (like v0.2.1). But everywhere a person actually sees the version — CHANGELOG.md, Cargo.toml, mapbox --version — it's shown without the v. So if someone copies the version from any of those and uses it to pin, it fails with a confusing error instead of a helpful one.

Might be worth a short note about this somewhere near the version/compatibility section.

Comment thread CLAUDE.md Outdated
Comment on lines +1 to +3
See [AGENTS.md](./AGENTS.md). That file is the source of truth for Claude Code
and every other agent tool; this pointer exists only so Claude-specific
tooling that looks for `CLAUDE.md` finds it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
See [AGENTS.md](./AGENTS.md). That file is the source of truth for Claude Code
and every other agent tool; this pointer exists only so Claude-specific
tooling that looks for `CLAUDE.md` finds it.
@AGENTS.md

Claude official doc decleared a way to work with Agents.md https://code.claude.com/docs/en/memory#agents-md

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mattpodwysocki this suggestion (using the @AGENTS.md import syntax per https://code.claude.com/docs/en/memory#agents-md) hasn't gotten a response yet — what's your take on it?

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, and it's worse than a documentation problem — so I've fixed the cause instead of writing the note. #29.

You're exactly right about which spelling people will copy:

$ MAPBOX_CLI_VERSION=0.2.1 …
curl: (56) The requested URL returned error: 403
mapbox-cli: could not read https://cli.mapbox.com/0.2.1/manifest.json

A bare 403 reads as "you are not allowed", which sends someone looking at their network or their permissions rather than at the version string.

Both installers now prepend the v when the value starts with a digit, so 0.2.1 and v0.2.1 both work and latest is untouched. Tested against the live channel and covered in both installer suites, including a case pinning latest — getting that one wrong would break the default install rather than an edge case.

I also documented MAPBOX_CLI_VERSION and MAPBOX_INSTALL_DIR, neither of which appeared in any .md in this repo. That came out of your #26 comment; see the reply there.

Leaving this PR's AGENTS.md alone, since with the prefix accepted there's no longer a discrepancy to warn about. Happy to add a line if you'd still like one.

@zmofei
zmofei requested a review from a team September 18, 2026 12:19
zmofei pushed a commit that referenced this pull request Sep 18, 2026
Reported on #27. The channel's directories are named `v0.2.1`, and
`MAPBOX_CLI_VERSION` went into the URL untouched — so the spelling a person
would actually copy failed:

    $ MAPBOX_CLI_VERSION=0.2.1 …
    curl: (56) The requested URL returned error: 403
    mapbox-cli: could not read https://cli.mapbox.com/0.2.1/manifest.json

Every place a version is read from shows it without the `v`: `mapbox
--version`, CHANGELOG.md, Cargo.toml. So the one spelling that worked was
the one nobody sees, and the failure was a bare S3 `403`, which reads as
"you are not allowed" rather than "no such version".

The review asked for a note about it. A note documents a trap; this removes
it. Both installers now prepend the `v` when the value starts with a digit,
which leaves `latest` — and any other channel name — alone, since only a
leading digit means a version number is being named.

Verified against the live channel: `0.2.1`, `v0.2.1` and `latest` all install
0.2.1. Two cases added to each installer suite, including one pinning
`latest` because getting that wrong would break the default install rather
than an edge case. Both confirmed to fail with the fix reverted.

**And the variable is now documented at all**, which is the other half of
this. `MAPBOX_CLI_VERSION` appeared in no `.md` in the repository — the
reviewer on #26 suggested using it, which is how I found that a reader had no
way to know it exists. It goes in the install section with
`MAPBOX_INSTALL_DIR`, which was also undocumented.

601 tests, both installer suites green, fmt clean, no broken anchors.
zmofei
zmofei previously approved these changes Sep 18, 2026
**The import, per @zmofei's suggestion, and he's right.** The docs are
explicit: "Claude Code reads `CLAUDE.md`, not `AGENTS.md`. If your repository
already uses `AGENTS.md` for other coding agents, create a `CLAUDE.md` that
imports it", with `@AGENTS.md` as the example.

The difference is not cosmetic. A prose "see AGENTS.md" is a sentence asking
Claude to go and read a file, which costs a tool call and may not happen.
`@AGENTS.md` is expanded into context at session start. So the file now does
deterministically what it previously only suggested.

The note below it is a block-level HTML comment, which the docs say is
stripped before injection — so it explains the mechanism to a human reader
without spending context on it.

**Four notes from @zmofei's review**, with one correction to his first.

He reported `cargo check --target x86_64-pc-windows-msvc` failing on
`ring`'s build script wanting `assert.h`. I could not reproduce that, and the
reason turned out to be worth more than the tip: `rustc` here resolves to
`/opt/homebrew/bin/rustc`, not rustup's shim, so the check fails earlier with
`can't find crate for core` and `rustup target add` reports "up to date"
about a toolchain that is not the one running.

Which means Homebrew's cargo does not read `rust-toolchain.toml` at all, and
the pin this repository relies on is inert in that setup. It only goes
unnoticed because Homebrew currently ships the same 1.98.1 the file asks for.
`rustc --version` printing `(Homebrew)` is the only tell. That is now
written down, since AGENTS.md otherwise says "rustup applies the pin on its
own", which is true of rustup and silently false here.

His conclusion stands either way — Windows-gated code cannot be checked from
a Mac, and the standalone-file workaround is the way to look at it.

`actionlint` and `mapbox --schema` are his too, unverified by me: no
actionlint on this machine to try it with.

CLAUDE.md joins `prose_files`, so the spelling guard covers it now that it
holds prose.

AGENTS.md is 209 lines, past the 200 the docs suggest as a target. Worth
knowing before the next addition — the answer then is probably
`.claude/rules/` with a `paths:` scope rather than more of this file.

597 tests, fmt and clippy clean.

@mattpodwysocki mattpodwysocki left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the slow reply on this one — and you're right, taken as-is.

The docs are explicit, and I hadn't read that section closely enough:

Claude Code reads CLAUDE.md, not AGENTS.md. If your repository already uses AGENTS.md for other coding agents, create a CLAUDE.md that imports it so both tools read the same instructions without duplicating them.

The difference isn't cosmetic, which is what I'd missed. A prose "see AGENTS.md" is a sentence asking Claude to go read a file — a tool call it may or may not make. @AGENTS.md is expanded into context at session start. So the file now does deterministically what it previously only suggested.

I kept a note under it as a block-level HTML comment, since the docs say those are stripped before injection — so it explains the mechanism to a human without spending context on it.

Your four notes are in, with one correction to the first

I couldn't reproduce the ring/assert.h failure, and chasing why turned out to be worth more than the tip. On this machine cargo check --target x86_64-pc-windows-msvc fails earlier:

error[E0463]: can't find crate for `core`
  = note: the `x86_64-pc-windows-msvc` target may not be installed

And rustup target add kept insisting it was already installed. The reason:

/opt/homebrew/bin/rustc     ← first on PATH
rustc 1.98.1 (Homebrew)
rustup default: stable-aarch64-apple-darwin

rustc resolves to Homebrew's, not rustup's shim — so rustup target add was configuring a toolchain that wasn't the one running. Which means Homebrew's cargo doesn't read rust-toolchain.toml at all, and the pin this repo leans on is inert in that setup. It goes unnoticed only because Homebrew currently ships the same 1.98.1 the file asks for. rustc --version printing (Homebrew) is the only tell.

That's now in AGENTS.md, because the file otherwise says "rustup applies the pin on its own" — true of rustup, and silently false here. Your conclusion stands either way: Windows-gated code can't be checked from a Mac, and the standalone-file-with-stub-consts workaround is the way to look at it.

actionlint and mapbox --schema went in as you wrote them — I have no actionlint here to verify the shell: claim against, so they're yours rather than mine.

The pwsh-tarball tip I left out, only because it's about getting a tool rather than about this repo, and the file is tight on space — see below. Happy to add it if you think it earns the line.

One thing worth knowing before the next addition

AGENTS.md is now 209 lines, past the 200 the same docs page suggests as a target ("Longer files consume more context and reduce adherence"). And with @AGENTS.md it all loads every session now, which is the point but also the cost.

So your offer of a follow-up PR is welcome, but I'd aim it at .claude/rules/ with a paths: scope rather than more of this file — that way workflow notes load when someone touches .github/workflows/, and Rust notes when they touch src/, instead of always.

Also added CLAUDE.md to prose_files, so the spelling guard covers it now that it holds prose.

597 tests, fmt and clippy clean. Ready for another look.

@mattpodwysocki
mattpodwysocki merged commit 5a2c9af into main Sep 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants