docs(agents): rebuild AGENTS.md and add the review loop - #200
Merged
Conversation
AGENTS.md becomes the real file; CLAUDE.md and .cursorrules symlink to it, inverting the previous direction. The instruction file goes from 301 lines / 2,259 words to 92 / 826 — it had become a changelog of the repo rather than an instruction set for working in it, with nearly every feature PR appending a paragraph. Architecture Overview and Linear Integration were 45% of the file and were already carried by README.md, docs/INDEX.md, and .husky/commit-msg. Coding rules move to docs/CONVENTIONS.md, Linear team IDs to docs/GUIDANCE.md; the tech-stack versions, scope tables, anti-rationalization table, and reverted-architecture narrative are deleted. Adds what was missing: that these programs sign transactions and move real funds, a funds-at-risk trigger list, and a Traps section for hazards that are not discoverable from the code (worktree placement vs knip, gh stack, log fields as a dashboard interface, soltag's silent empty-ABI degradation). Fixes the surfaces AGENTS.md now delegates to: docs/INDEX.md linked 13 of 28 TIBs and 2 of 6 bots, README.md named 5 of 9 packages, and both specialist agents cited a packages/abis that does not exist. Adds a .husky/pre-push guard so "never push to main" has a mechanism. TIB-2026-09-03 records the decision and the admission criteria; TIB-2026-04-16 gets an addendum, since it recorded the opposite intent.
Adds .agents/skills/review-loop/SKILL.md as the single home for one procedure with three parameterizations: TIBs, implementations, and PR-review responses. All three share the same engine — satisficing self-check, dispatch independent reviewers in the background, do the readability pass without touching semantics until they report, then dedupe, synthesize, and decide incorporate / drop / defer. Writing that engine into three command files would have guaranteed drift. TIBs additionally get a clean-room subagent (never a fork, which would inherit context) and a two-round Codex reviewer whose session id is recorded in a git-ignored docs/decisions/<stem>.sessions file, so the implementation review can resume the reviewer that already argued about the spec. AGENTS.md gains six lines: the satisficing check before presenting a plan or proposal, and a pointer to the skill. 99 lines, still inside the TIB's budget. Linear filing now routes to Bots with surface labels and no project, per docs/GUIDANCE.md; create-issue.md was sending everything to a Curator Backlog project. PR descriptions lead with intent and carry a src/test/docs/ci size table from git numstat plus tokei, which counts Markdown as prose so a docs PR does not read as thousands of lines of code.
knip exits 1 from inside a worktree nested under .claude/worktrees/ — knip.json's .claude/** ignore only applies to runs started from the repo root. Verified by running pnpm knip inside .claude/worktrees/unwire-v1-ci. The previous wording called that path sanctioned, which reintroduces the false-positive wall that masks real findings.
`.claude/worktrees/` was unanchored, so the pattern matched that segment at any depth. Run from inside a nested worktree, knip matched every file against its own repo's rule via the absolute path, concluded the whole tree was gitignored, found no entry points, and reported ~29 files plus a pile of devDependencies as unused. The wall of false positives is what buried real findings on PR #184. Anchoring to `/.claude/worktrees/` binds the pattern to the repo root, which is the only place the directory exists — git behavior is byte-for-byte identical. Verified: knip exits 0 inside `.claude/worktrees/unwire-v1-ci` with the anchored pattern and 1 without, `git check-ignore` still resolves, root knip still clean. Worktrees can now live inside the repo again, so the guidance shrinks to one line. Worktrees created before this lands carry the old rule in their own checkout and need a rebase onto main.
A ticket born from a review loop now carries Provenance > code-review and links the PR or TIB it came from, so what the loop keeps punting stays auditable. Provenance is deliberately separate from status: the status says the work is not being done yet, the label says where the ticket came from.
docs/CONVENTIONS.md carried generic when-to-comment boilerplate with nothing about length or what a docstring should contain. The substantive policy lived only in a Claude-specific user file, which Codex — a first-class reviewer here via the review-loop skill — never reads. Ports it: default to none, ~3 lines max, TSDoc only where there is real nuance and stating the guarantee rather than the algorithm, one home per explanation, inline comments only where code looks wrong without them and only with a citation. Reconciles pithy with quoter-bot's jsdoc:check — pithy is not absent.
Adds .devin/config.json with read_config_from.claude, which pulls in AGENTS.md rules (via the CLAUDE.md symlink), the review-loop skill, and .claude/commands as skills. Subagents are explicitly not imported by that mechanism, so .devin/agents/ carries four thin manifests that pin model and tool surface and defer to .claude/agents/<name>.md for the instructions — one home, no copied prose. Every manifest pins model: because an unpinned custom subagent silently falls back to SWE-1.6, which would quietly degrade the review loop rather than fail it.
The model rule is role-based, not session-based: Fable for anything touching a TIB, Opus otherwise. documentor pins fable; reviewer, protocol-engineer, and product-manager pin opus. The clean-room agent needs no pin — it is a subagent_general and inherits the session model, so starting a TIB session on Fable covers the whole TIB path. Also records the guardrail: an agent must never rewrite .devin/agents/ to match the model it is running on. Those manifests declare intent; mirroring the session into them dirties tracked files every run and makes committed state depend on who ran last.
The loop's independent reviewer is Codex on GPT Sol because it is a different vendor from whatever wrote the code — that independence is the point, not the model's ranking. A Devin session may have no codex binary and no ~/.codex/sessions/, in which case the loop would quietly fall back to the reviewer subagent, which is Opus. An Opus session reviewed by an Opus subagent is not an independent pass and nothing in the output says so. Adds an independent-reviewer manifest pinned to the codex model as the fallback, and pre-approves Exec(codex) so the preferred CLI path is not gated on an approval prompt. Requires naming which reviewer actually ran.
MISSION.md arrived as a copy from morpho-apps and still described Next.js, React, wagmi, @repo/web3, @repo/ui, a design system, Core Web Vitals, accessibility, VVRM, ActionFlows, and AI QA driving a real UI — none of which exist here. The North Star, guiding principles, pillars, and horizons transfer unchanged; the concrete instantiations did not. New bots come from @repo/bot-kit and @repo/swaps, bug reports reproduce against a pinned anvil fork rather than a preview deploy, currency means Node/viem/Morpho SDK rather than a Next.js major, and the data-flow question is discovery-to-broadcast rather than chain-to-UI. Pillar B is retargeted at the threat model that actually applies: these bots hold keys and broadcast with no human approving each action, so the risk is our own transactions — unbounded approvals, a broadcast that skipped simulation, bounds that stopped binding, a lost nonce cursor — and signing is a fail-closed boundary. Adds one success criterion with no analogue in an app repo: a bot that cannot act safely must halt loudly, because idle-because-healthy and idle-because-broken look identical from outside.
haydenshively
marked this pull request as ready for review
September 4, 2026 03:41
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Devin Review found 2 potential issues.
3 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
cashd
approved these changes
Sep 4, 2026
The devin CLI is installed now, so the slugs could finally be checked. Three were wrong and one was outright broken: - documentor had model: fable, which is NOT a valid slug — Fable 5.1 is the one family with no alias. Now claude-fable-5-1-high. - the specialists used the opus alias; now claude-opus-5-high, explicit about reasoning level, which is the thing that matters for a reviewer. - independent-reviewer used the codex alias (GPT-5.3-Codex, 400K context). Devin also offers gpt-5-6-sol-high — the same model the Codex CLI path uses — so both routes now review identically instead of one silently being a different, older OpenAI model. Records the sharper trap: devin doctor loads profiles and reports zero failures with a bogus model value, verified with model: fable. A misspelled pin degrades as silently as an absent one, so slugs get cross-checked against devin models list.
It passed on a malformed allowed-tools one-liner as well as on a bogus model slug — the check confirms profiles load, not that their contents mean anything.
Two open review findings from Devin on PR #200. tokei was invoked through xargs without -r. GNU xargs runs its command once even with empty input, and tokei with no paths scans the whole repository, so a deleted-only PR would have reported the repo's totals as the size of the diff. BSD/macOS xargs already declines to run on empty input and accepts -r as a no-op, so one portable flag covers both; verified empty input now invokes nothing and a real diff still measures. The .sessions path was anchored to docs/decisions/, which strands the record for a bot- or package-scoped TIB away from the TIB itself and defeats the sibling lookup the implementation review depends on. It is now derived from the TIB's own path, which is also why the ignore rule is recursive.
Parameterization C read 'First dispatch B's independent reviewer, then do B's own pass and synthesis' — an unconditional instruction to re-review on every PR-comment response. That over-corrected an earlier finding which said C never mentioned the reviewer at all: it went from silent to always, skipping the intended once. The independent pass runs once after the implementation, in B. The findings being answered in C are themselves an independent review, so another round on a routine comment-fix buys nothing. It is now the implementer's call, with a stated trigger — when the response stops being a fix and starts being an implementation — and a requirement to say which reviewer ran, or that none did and why.
Three places where addressing review feedback drifted from the stated rule. create-issue.md's worked example treated 'align lint config with morpho-apps' as a Morpho-owned dependency and punted it to the user. It is work in this repo, so it goes to Bots; the surface-to-the-user rule is about dependencies. Replaced with two examples that each demonstrate one branch correctly — an in-repo ticket with no fitting label, and a real dependency (blue-sdk) that does get surfaced. start.md marked labels optional. Labels are the routing mechanism that the no-projects decision depends on, and create-issue.md already stated them as expected, so the two commands disagreed. start.md also claimed the Linear agent 'routes tickets by label' — an unverified mechanism. It assigns projects; how it decides is not ours to assert.
C was written as a prohibition — 'do not dispatch another by default' — which says what not to do without saying what the sequence is. It is now the sequence, and the relationship to B is stated outright: same loop, ordering inverted. B implements, then asks for review and reads it. C reads the review, implements, and only sometimes asks for another. Because a round of implementation and review has already completed, C enters at the Engine's step 4 rather than the top: dedupe and decide on the findings in hand, implement what was incorporated, then dispatch a reviewer only when something substantial changed — with edits to the TIB or plan as the signal, since a moved intent means nobody has reviewed what now exists. Entropy is promoted from one of two trailing additions to the constraint governing all of C, and named against each step: which findings to act on, whether to implement at all, and how much new surface a fresh review would need to cover.
The 1-2-3-4 numbering asserted a sequence the parameterizations do not share. C is the proof: it opens on what was step 4, then implements, then conditionally does step 2 — so the numbers described A and B and quietly mislabelled C. The pieces are now named — Self-check, Independent review, Own pass, Synthesis — and each parameterization opens with its own explicit Order line. Synthesis keeps internal ordering because its parts genuinely feed each other; that is scoped to the piece rather than implied across all of them. The no-semantic-edits rule was riding on the old adjacency of steps 2 and 3. It is restated as what it actually is: a concurrency rule binding whenever review and own pass overlap, in any parameterization.
Consistency pass before merge, all three the same defect: a fact stated in more than one file, where only one copy got fixed. docs/GUIDANCE.md still described the .sessions record as docs/decisions/<tib-stem>.sessions — the root-anchored form Devin flagged and that was corrected in the skill and the documentor agent, surviving here in a third copy. It now derives from the TIB's own path like the others. babysit-pr.md restated Parameterization C rather than pointing at it, and had already drifted: it carried the old dedupe/synthesize/decide phrasing and omitted the implement and conditional-review steps entirely. It now points. documentor.md recapped Parameterization A's sequence, which A's own Order line owns. Trimmed to a pointer; the .sessions instruction stays, since that is an action the documentor takes rather than a restatement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
AGENTS.mdhad become a changelog of the repo rather than an instruction set for working in it — 301 lines billed on every request, of which Architecture Overview and Linear Integration alone were 45%, all of it facts thatREADME.md,docs/INDEX.md, and.husky/commit-msgalready own. This rebuilds it from first principles at 102 lines, gives every evicted rule exactly one home, and repairs the surfaces it now delegates to.Along the way it fixes a real bug:
knipfalse-failing inside nested worktrees, which had been worked around for a month by telling people to keep worktrees outside the repo.Changes
Instruction file —
AGENTS.mdis now the real file withCLAUDE.mdand.cursorrulessymlinked to it (inverting the old direction). Seven sections: orientation, Never, Funds at risk, Working, Done, Traps, and an Elsewhere routing table. Adds what was missing — that these programs sign transactions and move real funds — plus traps that were only ever in operators' heads.Review loop —
.agents/skills/review-loop/SKILL.mdis the single home for one procedure with three parameterizations (TIBs, implementations, PR-review responses): satisficing self-check, independent reviewer in the background, own readability pass without touching semantics, then dedupe → synthesize → decide incorporate/drop/defer. Writing that engine into three command files would have guaranteed drift.knip inside worktrees —
.gitignorehad.claude/worktrees/unanchored, so the pattern matched that segment at any depth; run from inside a worktree, knip matched every file via its absolute path, concluded the tree was gitignored, and reported everything as unused. Anchored to/.claude/worktrees/. Git behavior is identical.Linear — routing moves from a stale Curator default to the Bots team;
create-issue.mdwas sending every ticket to a dead Curator Backlog project. Deferred review findings now carryProvenance > code-reviewand link their originating PR or TIB.Conventions — coding rules evicted from
AGENTS.mdland indocs/CONVENTIONS.md, including a comment policy that was previously only in a Claude-specific user file that Codex never reads.Devin —
.devin/config.jsonimports the repo's rules, skills, and commands viaread_config_from.claude; that mechanism does not import subagents, so.devin/agents/holds thin manifests that pin model and tool surface and defer to.claude/agents/<name>.mdfor instructions. Models are pinned by role:fablefordocumentor(the TIB role),opusfor the specialists. The clean-room agent needs no pin — it is asubagent_generaland inherits the session model, so a TIB session started on Fable covers the whole TIB path.Two traps are documented rather than left to be discovered: an unpinned custom subagent silently falls back to a fast cheap model, and a Devin session with no
codexbinary would drop the loop's cross-vendor pass down to Opus-reviewing-Opus with nothing saying so. Henceindependent-reviewer(pinned tocodex) as the fallback, andExec(codex)pre-approved. Devin support is best-effort:fableis not a documented Devin identifier and could not be verified from here.Mission —
docs/MISSION.mdarrived as a copy frommorpho-appsand still described Next.js, React, wagmi,@repo/web3,@repo/ui, a design system, Core Web Vitals, VVRM, and AI QA driving a real UI. The North Star and principles transfer; the instantiations are retargeted at bots, and Pillar B now names the threat model that actually applies here — these bots hold keys and broadcast with no human per action, so the risk is our own transactions.TIB-2026-09-03records the decision and the admission criteria for anything added back;TIB-2026-04-16gets an addendum, since it recorded the opposite intent.Size
docscitokeiover the touched files: 171 lines of code, 3,476 of prose. It counts Markdown as comments, so the +962 is almost entirely documentation — the only executable changes are thepre-pushhook and shell snippets.Test Plan
CI covers lint, typecheck, knip, and tests. Three things it cannot check:
pnpm exec knip --directory .claude/worktrees/<any>exits 0 — the fix's whole point. Worktrees created before this branch carry the old rule in their own checkout and need a rebase first.readlink CLAUDE.md .cursorrulesboth printAGENTS.mdafter checkout —oxfmtruns over*.mdand a write-through would silently un-do the inversion.AGENTS.mdalone.