docs: add AGENTS.md — architecture, invariants and conventions - #66
Conversation
This repo had no instructions file of any kind. CLAUDE.md was gitignored and never committed, and the working copy was lost, so the only machine-readable rules about how to work here lived in tooling outside the repo. A fresh clone, an outside contributor, and every automated review tool saw none of it. That has a concrete cost. An automated reviewer recently filed the `::` wallet-id charset restriction as a bug, because nothing in the repo said the restriction is what makes cost-center derivation collision-free. Review tools cannot be handed this context out of band — a repo-root AGENTS.md is the channel. Contents: the architecture and the flow of a governed call; the invariants, each stated as a rule plus the concrete failure it prevents; the packages/verify zero-dependency parity contract with the mirrored-file list and the exact file-diff rule; conventions; real build and test commands; commit and DCO rules; a "what NOT to flag in review" table; and the known drift. Everything in it was verified against the code rather than carried over from memory. Three adversarial passes checked it afterwards: a fact-check that ran every documented command, an invariant-coverage pass that tested it against real incidents from git history, and a hygiene pass confirming no internal tooling leaked into a public file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Cam <cam@camwhiteus.com>
Four executable entry points begin with `#!/usr/bin/env node`, so the header sits on lines 2-3 rather than line 1. "New files open with" was not literally true for them, and a reader checking only the first line concludes the header is missing — which is exactly what the Codex gate did on this PR, reporting four false positives. The count itself was right and is unchanged: 113 of 118 files carry the header, the five that do not are all of `packages/acs-adapter/src/`. Also records that no file carries one of the two lines without the other. Raised by the Codex gate on #66 (P3). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Cam <cam@camwhiteus.com>
Codex Gate — review and triageTwo passes were run, because a scoped Pass 1 —
|
| Finding | Verdict |
|---|---|
| The Biome nested-root failure also surfaces sibling worktree config errors | Valid — fixed |
| "Run it from inside the worktree" is not universally safe | Valid — fixed |
"gh pr checks will never show one" is too absolute to verify from the repo |
Valid — fixed |
AGENTS.md is not on master yet |
Correct today, self-resolving on merge |
| Coverage-threshold block duplicates AGENTS.md | Already scoped to the reproduce-CI command only |
| Draft-PR gate duplicates AGENTS.md | Codex conceded the procedure belongs here |
The two Biome findings were the useful ones and reproduce exactly: ship/mobile-optimization and ship/site-perf both exit 1 on their own biome.json, on the deprecated recommended field — they predate the Biome 2 migration. CLAUDE.md now says the workaround holds for a worktree branched off current master, and that failing on its own config is a signal the branch needs rebasing.
(Those two stale worktrees and their remote branches are unrelated leftovers, flagged separately for cleanup.)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb21c4aef4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **Hard rules fail closed; soft rules stay lenient.** An indeterminate condition (missing or | ||
| mistyped guarded field) is treated as *satisfied* for a hard rule, so the guard still fires. |
There was a problem hiding this comment.
This guarantee does not apply to every condition: only gt/gte/lt/lte return "indeterminate" for missing or mistyped fields. Operators such as exists, contains, and regex return false, causing even a hard deny rule not to match. An agent relying on this statement could create a supposedly fail-closed string or existence guard that actually fails open, so the text must either limit the guarantee to numeric operators or require the evaluator to implement it consistently.
Useful? React with 👍 / 👎.
| There is a second, blunter layer with a governance consequence of its own: `MAX_REGEX_INPUT = 4096` | ||
| truncates the resolved value *before* matching, so a `matches` rule can never see past a field's | ||
| first 4096 characters. That is a control that fails **open** on long input — never write a `matches` | ||
| rule that has to catch something an attacker can push past that boundary. |
There was a problem hiding this comment.
regex policy operator
FieldOperator and evaluateFieldCondition recognize regex, not matches, and loadPolicies casts parsed JSON/YAML without validating operator names. A policy author following this wording can therefore write operator: "matches"; it reaches the evaluator's default false branch and silently disables the rule, including a hard deny intended as a security control.
Useful? React with 👍 / 👎.
| 8. **Audit first.** The `llm_call` event is appended to the hash chain **before** the budget commit | ||
| and before the ledger POST, so a `failClosed` deployment never settles an unaudited spend. |
There was a problem hiding this comment.
For streaming calls, finalizeStreamSettle commits budgetSpent and calls engine.postPendingSpend before appending the llm_call event; if that append fails under failClosed, only the receipt rejects after the spend has settled. Thus the unconditional claim that audit always precedes POST and prevents unaudited settlement is false for governed streams and should explicitly distinguish the non-streaming and post-delivery streaming paths.
AGENTS.md reference: AGENTS.md:L124-L130
Useful? React with 👍 / 👎.
| `--`-prefix match. The resolved wallet is echoed in both output modes, and **every echoed value goes | ||
| through `forDisplay` first** — an echoed flag value is argv, the caller may be an agent, and | ||
| picocolors adds SGR codes without sanitizing anything it wraps. |
There was a problem hiding this comment.
requireValue before claiming every echo is sanitized
When a space-separated value starts with -, requireValue embeds the raw value directly in its error (write ${flag}=${raw}...) without calling forDisplay. For example, a --parent value beginning with - followed by an ESC sequence reaches the terminal unchanged, so this statement currently hides a terminal-repaint path; sanitize that interpolation or qualify the claimed coverage.
AGENTS.md reference: AGENTS.md:L370-L374
Useful? React with 👍 / 👎.
1 participant
Summary
Adds
AGENTS.mdat the repo root — architecture, invariants, and conventions for anyone (or anything) writing code here.The repo had no instructions file of any kind. The
CLAUDE.mdthat once served this purpose was gitignored and never committed, and its working copy was lost, so it is unrecoverable. The practical effect is that a fresh clone, an outside contributor, and every automated review tool saw none of the project's rules — they lived only in tooling outside the repository.That has a measurable cost. On #63 an automated reviewer filed the
::wallet-id charset restriction as a P1 bug. It isn't one: excluding:from the parent charset is what makesparent::costCenterderivation collision-free, and widening it would have broken allocation rather than fixed it. The reviewer had no way to know, and no way to be told — scoped reviews can't take a custom prompt. A repo-rootAGENTS.mdis the channel that fixes this class of false positive permanently.Contents
packages/verifyzero-dependency parity contract: the mirrored-file table, the exactstrip()file-diff rule, and why divergence is CVE-class.jsextensions, Biome, Vitestglobals: false, test layout, source headers, comment stylenpx tsc -b --noEmitfails (TS6310)How it was built and checked
Every claim was verified against the code, not carried over from memory. Six readers harvested in parallel from the code, the CI/config, git history, and merged PR bodies, tagging each fact verified or asserted.
Three adversarial passes then reviewed the draft and found real defects, all fixed here:
resolveRatesclaim that was untrue on the headless path.exists-exception list that omittedcreateFundedBudgetWalletand would have licensed a double-funding regression, and a::guard stated as two of its three clauses — the omitted clause is the one that stopsa:::bfrom resolving to two different owners.AGENTS.mdis trackable and that the localCLAUDE.mdremains gitignored and absent from the index.Test plan
Documentation only — no code changes, so
lint,typecheck,testandtb-integrationare unaffected. The value is in the claims being true:npm run typecheck,npm run lint,npm test, the coverage invocation with all four CI thresholds, and the single-file vitest examplenpx tsc -b --noEmitwas confirmed to fail with TS6310 exactly as documentedstrip()parity rule was checked byte-for-byte against the test that enforces it, and all three mirrored file pairs were diffed