From 167fce88c4ff3818d6a6037950e91e99b3eee694 Mon Sep 17 00:00:00 2001 From: GigaHierz Date: Mon, 17 Aug 2026 21:23:40 +0100 Subject: [PATCH 1/3] chore: add issue/PR templates, shared rules, CLAUDE.md Co-Authored-By: Claude Fable 5 --- .claude/shared/engineering-rules.md | 192 +++++++++++++++++++++++++ .claude/shared/money-path-checklist.md | 22 +++ .claude/shared/tester-mode-pattern.md | 71 +++++++++ .github/ISSUE_TEMPLATE/bug_report.yml | 79 ++++++++++ .github/ISSUE_TEMPLATE/config.yml | 5 + .github/ISSUE_TEMPLATE/task.yml | 28 ++++ .github/ISSUE_TEMPLATE/user_story.yml | 60 ++++++++ .github/PULL_REQUEST_TEMPLATE.md | 77 ++++++++++ CLAUDE.md | 58 ++++++++ 9 files changed, 592 insertions(+) create mode 100644 .claude/shared/engineering-rules.md create mode 100644 .claude/shared/money-path-checklist.md create mode 100644 .claude/shared/tester-mode-pattern.md create mode 100644 .github/ISSUE_TEMPLATE/bug_report.yml create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/task.yml create mode 100644 .github/ISSUE_TEMPLATE/user_story.yml create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 CLAUDE.md diff --git a/.claude/shared/engineering-rules.md b/.claude/shared/engineering-rules.md new file mode 100644 index 000000000..cad5e9bf9 --- /dev/null +++ b/.claude/shared/engineering-rules.md @@ -0,0 +1,192 @@ +# Engineering rules — issues, PRs, tests, reviews, merging, closing + +One document, synced to every repo from `pm-kit`. Do not edit here — edit in `pm-kit/templates/.claude/shared/` and the sync opens PRs. + +Merged from four sources: the Mondeto team guide (merges/tickets/closing), the celo-composer overhaul playbook (30+ issues → 30+ PRs in 4 days), the hardening playbook (PRs #49–#82, security/money paths), and the agent-assisted-development standards (recent PR/review rounds — cited `(agent-std)`). Every rule has an incident behind it — cited as `(mondeto #N)`, `(composer #N)`, `(hardening #N)`. Where sources disagreed, the resolution and reasoning is in §9. + +**One-paragraph philosophy (agent-std):** treat the agent's output as claims to be verified, not results to be trusted. Measure instead of reason wherever possible, and pin the measurement in a test. State what the change does *not* do as clearly as what it does. When a guarantee appears in prose, a test must prove it on every path — narrative must never outrun code. When a reviewer pushes back, reproduce their finding before touching anything. The bugs that survive live at the seams (wiring, error paths, bounds), so verification means running the artifact, not just its units. + +The two rules that hold everything else together: + +1. **Evidence over assertion, at every stage.** Issues, PR bodies, reviews, and status comments are all claims. The only claims that survive unchanged are the ones somebody executed. State the *scope of your evidence*: "true of what I ran" is not "true of what a user gets" (composer). +2. **The record gets corrected, publicly, by whoever finds the error — including in their own work.** One uncorrected sentence in composer #423 caused three downstream fixes. Corrections are cheap; propagation is not. + +--- + +## 1. Issues + +**Verify before filing — "confirmed" means you ran it.** Distinguish "I ran this, here is the output" from "static reading suggests". Three exploration claims in the composer audit were wrong (a helper that "never returns true" worked; a "type-check failure" didn't exist; two "unwired" providers were wired) and one shipped into #423 and two docs PRs. Facts about the outside world get checked at the source: token addresses via `eth_call` not memory, package APIs against the installed package, advisories against the advisory API. Composer #388 contained fabricated contract addresses with correct-looking prefixes. + +**Name the defect in the code path that serves users, not where you noticed it.** Mondeto #182 reported a fee bug in the log-scan route, which only runs when the subgraph is unconfigured — never in production. The production path had the identical defect. Ask: is the location I found it the location users hit? + +**One fix-unit per issue, one priority per issue.** Cluster by *fix boundary*, not observation boundary: one ticket per thing one person fixes in one pass over one file-set, with a checklist inside. **Same-diff test:** if two tickets would be closed by the same diff, merge them (composer #416: one ticket, nine locations, one fix — right shape; four docs tickets closed by one PR — wrong shape). **Different-schedule test:** if two items in one issue would be scheduled or prioritised differently, split them — bundling a live defect with low-priority hardening makes the defect inherit the low priority (hardening #47 → #81). **One symptom is not one defect:** mondeto #196 conflated a blank map (parse failure) with a freeze (main-thread block); work on one looked like progress on the other. It became #225 + #196 + #226. AI makes finding cheap — findings outnumber fix-units ~4:1 — so cluster before filing, never map finding → ticket 1:1. + +**Anatomy of a good issue:** what happens (exact commands + real output, or user steps + observed) → root cause (`file:line`) → impact (who hits this doing what; for security/money, what the attacker gets — hardening #54 said "quota theft, a DoS lever against settlement, lands in rotated container logs and pasted debug output", which is what got it prioritised) → suggested fix → version/commit tested. Use the issue forms in `.github/ISSUE_TEMPLATE/`. + +**State non-goals, dependencies, and what is explicitly not a blocker.** Mondeto #201's single sentence "Explicitly not a blocker" kept it out of two sprint discussions. #215/#193 both name who owns the shared file. #216 says "Depends on #215" in the body so the ordering survives without memory. + +**Acceptance criteria as checkboxes; separate code work from ops work.** Hardening #54 lists "rotate the credential / recreate the container / purge captures" as their own boxes, so a code PR can close its half while the ops half stays tracked (#72: "this stops the repo adding to the exposure; rotation actually closes it"). + +**Say how we'd know it's fixed.** Before asking a reporter to retest, have a way to answer yourself. Mondeto #196 couldn't be measured — a client that fails to parse never fires analytics — so two support reports were the entire evidence base. Instrument first (#226), then ask, or better, just look. + +**Record negative results precisely or not at all.** Composer draft #204's "tried and did not work" table said `transpilePackages` doesn't work; it does — it was aimed at packages that don't publish the failing code. Record *why* it failed precisely enough to tell "doesn't work" from "applied wrong". When unsure, write *attempted, outcome unclear*, never *ruled out*. + +**Write the defect as a mechanism, with measurements** ("225 of 574 responses were template output") — numbers in the issue become pinned tests in the fix (agent-std). **Mark open questions as open** ("number left to implementer") so the implementer surfaces the judgement call instead of guessing silently. **Pin prior art to a commit SHA**, not a branch — branch links rot. Acceptance checklists include documentation requirements so they can't be dropped silently. For audits, split into lettered fixes (A–G) mapping 1:1 to PRs, with a status table on the parent issue. A root cause found during unrelated work (a CI flake, an infra gap) gets its own issue "so it outlives the PR". + +**When your issue turns out wrong, correct it in-thread:** what was wrong, what's true, and who downstream inherited it (composer #423 named the two PRs). Retitle when scope narrows (composer #463). One home per work item. + +**Priority lives in labels** — `priority:critical|high|medium|low` — not board fields (the shared DevRel board's Priority field has zero options; it is impossible to mark anything high there). `priority:critical` = money correctness, security, or user-visible wrong data. Don't inflate it. Add every new issue to the board when filed, not in batches. + +--- + +## 2. Pull requests + +**Branch → PR → squash merge into `main`. Nothing goes straight to `main`.** Branch names: `/-`, slug naming the *problem, not your solution* (`lena/196-minipay-freeze`); bots keep their prefix (`renovate/...`). (Resolution of a three-way disagreement — see §9.) + +**The PR title is the commit on `main`** (squash with PR title). Conventional Commits, scoped, imperative, stating the *outcome*: `fix(buy): block buying ocean pixels via long-press inspect path`, not `fix buy bug`. + +**Smallest correct diff, one concern per PR.** Composer #427's two-line quote fix unblocked six PRs. Big mixed PRs stall behind their weakest part. If it builds on another PR, say so in the first line, name the base, give review order, offer to rebase (hardening #76: "Stacked on #75. Merge #75 first and this retargets cleanly."). Independent PRs say "branched off main, independent of my other open PRs" (#79). Keep stacks shallow (depth ≤ 2); with squash merges the second-lander rebases with `git rebase --onto main ` after the base merges. + +**Every sentence in the body is a claim that will be adversarially checked.** Composer bodies that overclaimed all got caught ("only one place interpolates the name" — a second in JS; "the suite would have caught #399" — its check was a no-op). Write bodies you'd bet on, with **verification evidence as commands + output**, not "tested ✓". Include the **mutation count** (§3). + +**Surface judgement calls the spec left open as explicit, reversible decisions** ("named constant, one-line change if you want it more sensitive"). Never smuggle product decisions into bug fixes; if one is bundled, flag it for the maintainer to keep or drop ("happy to drop it; the fix stands without it"). Push back on the spec when the evidence disagrees, *with the evidence* — correcting an issue's overstatement is a contribution (agent-std). + +**Say what the PR actually does, and what it does NOT do — with numbers where possible** ("blocks repeats, not first submissions — roughly 190 of the 225"). Known accepted limitations are pinned in a test, not left implicit, so a future change has to confront them. Mondeto #224 said "data layer only" and also retired a product tab, rewired `/ranks`, and rewrote FAQ answers — reviewed under a label that said it wasn't there. Hardening #75's "what this does not buy" section ("whoever controls the endpoint completely can forge the receipt too; the independent second source is the follow-up, and this is not it") prevents a phrase like "verified on chain" from stopping people asking questions. + +**`Closes #N` is a contract: only when the entire issue's acceptance criteria are met. Otherwise `Refs #N`** and list which boxes this closes. Composer edited #448/#454 mid-flight for this; mondeto #209 got it right in prose and was overridden anyway (§6). + +**Never delete a wrong claim from a PR body — strike it through with a pointer to the correcting commit.** The visible correction is the trust signal (agent-std). Because we squash-merge with PR title + body, the PR body *is* the commit message on `main`: rationale, verification performed, known limits, and `Closes`/`Refs` trailers all live there. Keep agent co-author trailers; they're honest and cost nothing. + +**For a stack, verify the merged union:** merge all open branches into one tree locally and run the suite — each PR green individually says nothing about the combination. **When you touch CI, prove it in both directions:** link one green run and one deliberately red run. + +**Reproduce before you fix, fix the root cause, collapse duplicated facts.** Hardening #77 wrote both attacks as tests, ran them red on main, then green. Composer's wallet-provider mess existed because one fact lived in four places and the copies drifted; #461 moved it to one module. If your fix restates a fact somewhere, ask where else it lives. State the trade-off that picked the design — "a missing file fails loudly at once, a missing variable misbehaves silently at runtime" (hardening #74) is the strongest one-line justification a PR can carry. + +**Flag breaking internal changes for parallel work** (hardening #77's signature change broke #79 — the note made it expected). **Know your conflicts before the reviewer does:** when PRs touch shared files, say so, propose a merge order, pre-write the resolution hunk (composer #428/#450). Whoever merges second rebases. + +**Runbooks, README, `.env.example`, examples, and error strings are part of the diff.** Grep for the old instructions (hardening #74's README still pointed at the old secret layout; stale docs "asserting the property an issue exists to create are worse than silence, because they stop anyone checking" — #76). Runbook steps get reviewed like code (#74's was missing `--build`). + +**Ask before public or irreversible actions** — filing on external repos, posting from the org, force-pushing shared branches, deleting branches, anything outward-facing. Track the decision as an owned issue (hardening #80), not a PR thread. **This applies doubly to agents: propose, never execute, anything outward-facing.** + +**Lockfile PRs — the rule that burned Mondeto twice (#162, #212):** never merge two lockfile-touching PRs without rebasing the second. Merge one → wait for `main` to update → rebase the next and **regenerate the lockfile, never hand-resolve** → wait for green on the rebased head → merge. Renovate only rebases when it next runs. (The ruleset's "require branch up to date" now enforces the rebase mechanically; the regenerate-don't-hand-resolve part is still yours.) + +--- + +## 3. Tests & verification + +**Every change ships with the test that would have caught the bug** — the assertion that fails on pre-fix code, not merely a test that passes. Composer #461 v1 broke every MiniPay scaffold while its tests stayed green because the fixture asserted components existed but not that the manifest kept their dependencies. + +**Cover the seams, not just the units.** Pure-function tests are necessary, not sufficient — the bugs that survive live at the wiring: response bodies, query bounds, component props, error paths, config parsing. Every PR has at least one test *through* the seam it touches (route in → response out; CLI as subprocess; component render) (agent-std). **A stated guarantee needs a test on every path** — if prose says "the error response never reveals X", a test asserts it on the *rejected* path, not just the accepted one. **Measured over reasoned:** any threshold, constant, or "this can't happen" is justified by a measurement pinned in a test, never only in a comment. Edge classes to check every time: Unicode/non-Latin input; empty input; the band between two thresholds (clears one floor, misses another); exact boundary values (`>=` vs `>`); concurrent duplicate requests. + +**Error paths are an attack surface.** Audit what every 4xx/5xx body leaks — exact scores are gradient oracles, flags are detection oracles. Decide per check whether it fails open or closed, write it down, verify the code matches. **Reads on write paths are bounded at the query** (`take(limit)`), never collect-then-slice — a fail-open catch around an unbounded read silently disables the check for exactly the heaviest users. + +**Config safety:** no silent fallbacks for critical config — unset or unrecognised values are errors, not defaults (a quiet default has shipped the wrong environment to production). Env values compared exactly — set with `printf`, never `echo` (stray newline). Build-time-inlined vars (`NEXT_PUBLIC_*`) need a fresh build; dashboard edits alone do nothing. Never print secret values — check presence, not content; keys never in argv, shell history, or committed files. + +**Mutation-test the fix and state the count** ("disabling receipt verification turns 5 red" — hardening). A suite equally green with and without the fix proves nothing. + +**Pair every absence-assertion with a control.** "Nothing bad happened" must run beside "something happened at all", or it passes against code that never ran (hardening #75 caught four vacuous passes this way). **Check that "never happened" assertions can fail** — hardening #82 asserted `settleCalls() === 0` on a path where the counter can't increment. Make it fail by hand first. + +**Fixtures are real captured data.** Mondeto #220's classifier tests asserted on `'execution reverted'` — a bare fragment that never occurs; real viem errors carry a `Docs: https://viem.sh/...` line, which the rule matched on `http`, so real reverts were filed as network errors. Green tests, wrong behaviour. + +**Test doubles honour their inputs.** A fake `getLogs` that ignores the filter it's handed silently turns every test depending on that filter into a vacuous one (hardening #76, #78). + +**Assert at the boundary where damage happens** — the recorded webhook POST body, not stdout, "because a leak that only reaches Slack never appears in the console" (hardening #72). **Verify deployment claims from inside the running system**, and test the existing-state case (populated DB, root-owned volume), not just fresh state (#74, #77). + +**Paths that cannot be faked get run in production with a restricted audience — tester mode.** Staging proves code against a mock; it cannot prove real gas leaves the real treasury. Two DB flags (account capability + entity audience), server-side gating at every surface (listing, lookup → 404, join, in-flight reads, realtime, aggregates), and everything downstream flag-blind so the tester run is the identical pipeline with real (small) stakes. Mini-quiz's first native-CELO payout ran this way and surfaced three bugs (#19–#21) no offline test could reach. Verify at the source of truth, not the UI. Pattern and porting checklist: `tester-mode-pattern.md`. + +**Anything that decides who gets paid gets compared byte-for-byte against the other side** — contract, on-chain read, the other route (mondeto #48, #224 where the comparator was identical but the block window wasn't; #223 checked fee arithmetic against `feeRate()` live on all eight map contracts). Money/security diffs additionally run `money-path-checklist.md`. + +**We only test MiniPay, so browser regressions go unnoticed.** Mondeto #221 broke `mondeto.app` in every browser for four days. Wallet/provider tree changes get loaded in a normal browser too. + +**A green check is only as good as what the check runs.** Composer's CI was secret-scanning only for days — every broken PR was green. A "TypeScript check" resolved to npm's placeholder package `tsc` and certified 21 syntax errors. Know what gates actually verify; the CI in `pm-kit` fails without lint/test scripts and uses `--no-install` for that reason. **A green check reflects the commit it ran on** — mondeto #206's green run predated the bump it needed to guard. + +--- + +## 4. Reviewing + +**Review by attempting to refute, with the code running.** Reading diffs catches style; running catches a fix that silently stripped a template's wallet stack, a tarball shipping the files a script existed to exclude, a `tsc` that wasn't TypeScript (composer). **Tier it — uniform max-rigour was the real cost driver in the composer overhaul:** + +- *Mechanical* (renames, quoting, doc paths, green dep bumps): read + confirm what CI ran. No behaviour pass. +- *Logic, new surface, funds/security/release, wallet tree*: full pass — run the suite on the branch and prove tests can go red; build the real artifact (for a scaffolder, the generated project *is* the product); click through the changed surface once (dev server or the CLI command a user would run). Optionally a cloud multi-agent review first as a breadth pass — treat its output as claims to verify, and it never replaces the behaviour pass. + +**Verify claims against code, never against the description.** Hardening's descriptions were excellent and still wrong in places (#76 mutation table, #75 "nothing on chain could spoof this"). Half-wrong claims: identify the right half (composer's "both files wrong" — one was correct). **Verify the feedback itself before implementing it** — reviewers are claims too; two composer docs PRs implemented an incorrect issue premise that 30 seconds of grep would have caught. + +**Review against current `main`, on the head SHA you think you're reviewing.** Branches go stale in a queue; `gh pr diff` served stale content twice. **When CI is red, first ask what moved** — hardening #73's runner had a floating Node version, so main went red with no commit and every open PR looked guilty. + +**A PR description is testimony, not evidence** — the review's job is to find where the narrative outran the code. **Recompute the math:** pinned constants, statistical formulas, thresholds — rederive independently before trusting them. Probe the standard edge classes and the fail-open/fail-closed direction of every guard; audit error responses for leaks; check read bounds on write paths. **Separate CI signal:** say which failures are code and which are infra/permissions. **Name what the PR got right** — a request-changes that ends "one round of targeted fixes, not a rethink" gets a better resolution round than a bare defect list; every finding includes the shape of the fix ("one line: strip the flag when block is true"). **For payment paths, auth, or anything moving money: adversarially verify findings** — a second independent pass attempting to refute each — before posting; plausible-but-wrong findings burn author trust and review rounds (agent-std). + +**Independently confirm the highest-severity finding before acting** (both hardening HIGHs were re-verified line-by-line). A wrong HIGH burns trust; a confirmed one with `file:line` and an attack script is undeniable. + +**Every finding: severity · `file:line` · one-sentence defect · concrete failure scenario · suggested fix.** "The rate limits interact badly" is not implementable; "both throttles build the key `ip:`, `take()` refills at the calling tier, so alternating requests degrades 10/hour to 60/min — namespace them" is a 20-minute fix. **Say what you verified as sound** — it tells the author what not to touch and proves you read the code. + +**Verdicts:** `APPROVE` (zero open findings, however small) / `REQUEST-CHANGES` (anything else). There is no approve-with-nits: a PR is not mergeable while any finding is unresolved. A finding resolves as exactly one of: a fix commit on the PR, or — only when genuinely out of scope for the PR — a follow-up issue *filed and linked from the review*, never merely suggested. "Could be a new ticket" / "out of scope" in prose, with no issue behind it, leaves the finding open; deferred work always lands in a filed issue, otherwise it evaporates (composer #452, #460). **Review the tests as hard as the code:** if nothing realistic would make the suite go red, it's worse than none — it certifies broken output (composer #447 v1). + +**For a queue of PRs, build the conflict matrix mechanically** (`git merge-tree`) and publish the merge order; name the merge-order hazards (shared files, squash retargeting of stacks, signature changes) so the second merger expects the conflict. Sequenced merges kept every intermediate `main` working across ~30 composer merges. + +--- + +## 5. Receiving feedback + +The review round is where contributor quality is actually measured. In order (agent-std): + +1. **Reproduce before touching anything.** Run the reviewer's scenario and confirm it. If you can't reproduce, say so plainly — and still fix on the evidence if the mechanism is sound. Never implement a fix for a finding you haven't understood: a plausible fix for a misdiagnosed problem is two bugs. +2. **Fix, then audit your own fix** the way the reviewer reviewed the original. If the audit finds a bug in the fix, disclose it unprompted. A fix that silently changes scope is a new unreviewed change. +3. **Report what the fix taught** — not "fixed"/"done": what you reproduced, what changed, what it revealed about adjacent code. If your earlier framing made the bug look smaller than it was, correct the record. +4. **Push back only with a measurement** — an on-chain read, a header trace, a control run — not intuition. +5. **Defer ownership calls.** If a finding opens a design question that belongs to someone else, present options with trade-offs and ask; time-box it ("say what shape you want and I'll push it today"). Don't grow a tightly-scoped PR on your own judgement. + +**Answer every review point explicitly: `FIXED` / `NOT-FIXED` / `DISAGREE-because`.** Responses that silently addressed some points forced full re-reviews of everything (composer). Every review ask ends as one of: a commit, a filed follow-up issue, or an explicit "won't do, because…" — never silence. + +**Never respond with a silent push.** Push, then comment what changed and which findings it addresses (composer's early rounds were silent force-pushes; nobody could tell what moved). With squash merges you may rebase/force-push your own branch freely — the *comment* is the rule. + +**When review proves your fix wrong, the model response is composer #461's revision:** adopt the diagnosis, encode why in the code itself ("Caught in review on #461" now lives in a doc comment), add the missing assertion, say what you verified. Turnaround beat defensiveness by a day. **Add the test the reviewer says is missing, especially when it would have caught the finding** — each hardening HIGH came with the exact test gap that hid it. + +**Answer direct questions in PR threads promptly** — an unanswered go/no-go blocks the author invisibly (hardening #72). If it needs a decision process, convert to an owned issue and say so. + +--- + +## 6. Merging & closing + +**Merge what's ready promptly** — an approved PR sitting unmerged makes every stacked PR staler. Keep stacks rebased after the base merges. Call out schema migrations in merge notes. + +**Approvals don't survive a re-roll.** Mondeto #89 carried a sign-off for pnpm v10; Renovate re-rolled it to v11.5.3 with the approval still sitting there. (Ruleset dismisses stale approvals on push now — but a re-approval is a re-review, not a click.) + +**After merging, check what closed.** Mondeto #196 closed one second after #209 merged, even though #209's body said in bold it does not close #196 and its footer read `Closes #208. Refs #196.` — a Development-sidebar link outranks anything in the body. It stayed closed five days while the remaining work lived only in a draft PR. Rules: **sidebar links close issues; prose does not stop them** — unlink what you don't intend to close. **A ticket is done when its acceptance criteria are met, not when a related PR merges** — re-read them before closing. **If closing orphans remaining work, file the successor first** and link it from the closing comment. Run `/post-merge ` after every merge until it's a habit. + +**Close issues with evidence, not "done":** name the fixing PR and what was verified. Close stale issues and supersede stale PRs with the same specificity — thank the contributor, cite the superseding commit, invite fresh work (one such contributor returned with a mergeable PR the next day). **Every open PR carries a current status:** merged, reviewed with verdict, or what it's waiting on. + +**Closing a PR without merging — capture what it proved.** Unmerged work often contains proven pieces (a working flow, validated thresholds). Identify what's reusable, capture it in the issue that carries the work forward with file links pinned to the PR's head SHA, close with a comment saying why and where the value went, keep the branch unless there's a reason to delete (agent-std). **When CI goes red:** diagnose to root cause, prove it with a control run (no code change), file the diagnosis as an issue so it outlives the PR. Policy calls (retries, concurrency limits) go to the maintainer, flagged — not decided unilaterally. + +**Versioning:** the version lives in the repo, release tooling reads it, CI verifies tag == `package.json` and never invents versions. Changelog entries land with the change. + +--- + +## 7. What is enforced by the platform vs. by discipline + +Enforced (ruleset + repo settings from `pm-kit/protection`): PR required · 1 approval · stale approvals dismissed on push · required `ci` check · **branch must be up to date with `main`** · no force-push · no deletion · squash-only · PR title = commit. Everything else in this document rests on you. Treat a green check as information, not permission, until you know what it ran. + +--- + +## 8. The loop + +``` +observe (run it) → file (claims + evidence) → fix (smallest diff + the test that fails pre-fix) +→ review (attempt to refute, running; tiered) → revise (point-by-point, announced) +→ merge (in published dependency order) → verify end-to-end on main → check what closed → correct the record +``` + +Calibration: composer day 1, zero of six templates built; day 4, seven of seven build end-to-end with CI gating on a real scaffold suite. The delta was verification applied uniformly to everyone's claims, including our own. + +--- + +## 9. Where the sources disagreed — and what we chose + +- **Branch naming** — three-way: kit v1 `feat/…`, Mondeto `/`, agent-std `type/issue-slug`. Chose `/-`: the type already lives in the Conventional-Commit PR title (so a type prefix is redundant), the handle answers "whose is this" for cleanup and bots (renovate/ already does this), and agent-std's issue number is worth keeping — it links branch → ticket without a lookup. Best of each; the one thing dropped is the redundant type. +- **"Squash to logical commits whose messages carry rationale and `Closes` trailers" (agent-std) vs squash-merge-to-one-commit (Mondeto, ruleset).** The repo squashes with PR title as subject and PR body as message, so the *PR body* is where rationale, verification, limits, and trailers live — the same content agent-std wants, in the place the platform actually preserves. Intermediate commits on the branch can be whatever helps you; they don't reach `main`. +- **PR template shape** — agent-std proposes What & why / Scope-not / Judgement calls / Verification / Merge order. Ours already had the first two, verification, and stacking; the missing piece was **Judgement calls**, now its own section. Nothing dropped. +- **"One issue per problem" vs "cluster findings"** — composer §1 vs its own §7 retrospective, and hardening's "one priority per issue". Not actually opposed: cluster by *fix boundary* (same-diff test) and split by *schedule/priority* (different-schedule test). Both tests are in §1. +- **Required review-thread resolution (kit v2 ruleset) vs "approve-with-nits means mergeable now" (hardening).** Both superseded. The ruleset rule stays dropped — mechanical thread resolution is click-blocking, not review quality. But the approve-with-nits verdict is gone too: no approval while any finding is open, however small. Every review point ends as a fix commit, a *filed* follow-up issue, or an explicit won't-do agreed in the thread (§4, §5). +- **"Require branch up to date" (mondeto lockfile incidents) vs the parallelism tax of many small PRs on shared files (composer §7).** Kept strict up-to-date — the lockfile incidents broke production twice, the tax is a click and a CI run. If the queue gets deep (>~5 concurrent PRs on shared files), enable GitHub **merge queue**, which performs the up-to-date test automatically and removes the clicks. Auto-merge on. +- **Squash-only vs stacked PRs.** Squash makes stacks slightly more work (second-lander rebases `--onto main`). Kept squash — the clean linear log and title-as-commit are worth more than stack convenience; keep stacks ≤ 2 deep and prefer independent branches off `main`. +- **Priority labels** — kit v1 `P0/P1/P2` vs Mondeto `priority:*`. Chose `priority:critical|high|medium|low` (already in use, and the word "critical" carries its own definition; P0 doesn't). +- **Force-push** — Mondeto "never respond with a silent force-push" vs squash-merge culture where rebasing your own branch is normal. Resolution: force-pushing *your own PR branch* is fine; the rule is *announce what changed*. Never force-push shared branches or `main` (ruleset blocks it anyway). +- **Review effort** — composer's "both passes for every substantive review" vs its own retrospective "uniform max-rigour was the real cost driver". Chose tiered (§4); a real CI suite is what makes the light tier safe, which is why CI came first. diff --git a/.claude/shared/money-path-checklist.md b/.claude/shared/money-path-checklist.md new file mode 100644 index 000000000..3770bffb4 --- /dev/null +++ b/.claude/shared/money-path-checklist.md @@ -0,0 +1,22 @@ +# Recurring-defect checklist — run against any security- or money-path diff + +Each item caught at least one real finding (hardening PRs #49–#82, mondeto, agent-assisted-dev rounds). Reviewers and `/review-pr` run this item by item; authors run it before opening the PR. + +1. **Self-referential verification** — is X checked against a value derived from X? (#75: receipt's token address compared to the log's own address.) Anchor checks to config/allowlists, never to the untrusted input. +2. **Defaults on the money path** — any `?? fallback` where the fallback has value? (#75: unknown token `?? "USDC"`.) Unknown means reject-and-alert, never a default. +3. **Shared keyspaces** — do two limiters/caches/locks build the same key? (#82: `ip:` shared by two throttles with different tiers.) Namespace per purpose. +4. **One-shot checks** — does a safety check latch on first failure and never retry? (#75: decimals check latched false on a transient RPC error.) Distinguish "refuted" from "unreachable"; only refuted may latch. +5. **Unbounded wedge + alert flood** — can one poisoned item pin a cursor or queue forever, and does its alert re-fire every tick? Bound retries, dedup alerts. (#75/#78; two wedge incidents in that codebase.) +6. **Silent-stall siblings** — for every loud failure path, is there a quiet branch that resets the failure counter? (#78: pending-log branch stalled with no alert while the tick "succeeded".) +7. **Upgrade path on existing state** — migrations against a populated DB, deploys against a root-owned volume, compose changes on a box mid-layout-change. Fresh-state testing proves nothing about the box you have. (#74, #77.) +8. **Cap semantics** — does the limit bind the actual resource or a client claim? (#82: `Content-Length` checked, chunked bodies still fully buffered.) Name which layer holds the real bound. +9. **Docs/code drift** — do README, bootstrap, examples, and error strings still describe the world this diff creates? (#74 README; #76's stale "recipient is not the treasury".) +10. **Test-double honesty** — does every fake honour the arguments it receives, and can every "never happened" assertion actually fail? (#76/#78 filter-ignoring fake; #82 vacuous counter.) +11. **Payout parity** — anything that ranks or pays: compared byte-for-byte against the other side (contract `fee = price * bps / 10000`, live `feeRate()`, the other route), including the *window* handed to an identical comparator (mondeto #48, #223, #224). +12. **Fee omission in every path** — if a fee/discount is subtracted in one code path, is it subtracted in all paths that serve users? (mondeto #182: log-scan path fixed, subgraph path — production — identical defect.) +13. **Error-body leakage** — what does each 4xx/5xx reveal? Exact scores are gradient oracles; flags are detection oracles. Fail-open or fail-closed decided per check, written down, and matched by the code (agent-std). +14. **Read bounds on write paths** — every read is bounded at the query (`take(limit)`), never collect-then-slice; no fail-open catch around an unbounded read (agent-std). +15. **Silent config fallbacks** — no `?? default` for critical config; unset/unrecognised is an error. Env compared exactly (`printf`, not `echo`); build-inlined vars need a rebuild; secrets checked for presence, never printed (agent-std). +16. **Guarantee-on-every-path** — for each prose guarantee ("never reveals X", "payment flow untouched"), point at the test asserting it on the *rejected/failure* path (agent-std). +17. **Threshold bands and boundaries** — the band between two thresholds, `>=` vs `>` at exact values, Unicode/empty input, concurrent duplicate requests (agent-std). +18. **Has it run in anger?** — for a path that moves real money / sends real messages / writes to a third party: was it exercised in production via **tester mode** (restricted audience, identical pipeline, small stake, verified at the source of truth) before public launch? If the app has no tester mode yet, that's the first ticket — see `tester-mode-pattern.md`. And `grep` the audience flag: it must not appear downstream of access control. diff --git a/.claude/shared/tester-mode-pattern.md b/.claude/shared/tester-mode-pattern.md new file mode 100644 index 000000000..3b2232fa4 --- /dev/null +++ b/.claude/shared/tester-mode-pattern.md @@ -0,0 +1,71 @@ +# Tester mode — run real paths in production with a restricted audience + +Synced from `pm-kit`. Generic pattern; the worked example is mini-quiz (see end). + +## Why it exists + +Some paths cannot be meaningfully faked: anything that moves money, spends gas, sends a real message, or writes to a third-party system. A staging environment proves the code typechecks against a mock; it does not prove that real gas gets spent from the real treasury and the real receipt comes back. Tester mode lets you run the *identical* production pipeline with real (small) stakes, visible only to a handful of internal accounts. + +It has already paid for itself: the first native-CELO payout in mini-quiz was run this way (#15) and surfaced three bugs (#19, #20, #21) that no offline test could have found — all three lived in the gap between "the code typechecks" and "real gas was spent from the real treasury". + +## The model: restrict the audience, never the behaviour + +Two flags, both **database columns** (not config, not env — toggling must never need a deploy): + +| Flag | Lives on | Meaning | +|---|---|---| +| capability flag, e.g. `isTester` | the **account** | this account may see and act on tester-only entities | +| audience flag, e.g. `testerOnly` | the **entity** (quiz, campaign, map, product, drop…) | visible only to tester accounts | + +Toggle both from the admin surface. Index the audience flag together with whatever the public listing filters on, so the public query stays cheap. + +## Enforcement: server-side, at every surface + +The frontend never decides. Every check runs in the API, and **each surface is gated independently** — one leaked room code or one missed UI condition must expose nothing. The honest work of porting this pattern is *enumerating the surfaces*; the checks themselves are one-liners. The standard set: + +| Surface | Behaviour for non-testers | +|---|---| +| Listing / feed / search | filtered out **in the query**, not post-filtered | +| Direct lookup by id/code/slug | **404, not 403** — existence is part of what's hidden; probing confirms nothing | +| Join / mutate / submit | rejected with a specific error code | +| In-flight reads (state, results, progress) | **re-checked on every call** — no riding along mid-flow with a leaked code | +| Realtime (SSE / websocket / push) | checked at subscribe time | +| Aggregates (leaderboards, stats, analytics, exports) | **decide explicitly per aggregate** — see the gap below | +| Notifications / emails / social posts | tester entities never trigger public-facing sends | +| Deep links / OG previews / sitemaps | 404 / excluded | + +Identity comes from the same auth the endpoints already use — there is no separate "tester API". + +## The part that makes it valuable: downstream is flag-blind + +Scheduler, scoring, payout, settlement, treasury, messaging workers contain **zero** references to the audience flag. A tester run is not a simulation; it is the identical pipeline with real prizes, real gas, real transfers. The only restricted dimension is *who can see it*. + +Keep it this way. A sandbox branch inside the payment code means the one path you most need to prove — real money moving — is exactly the path a test never runs. **Do not add tester awareness downstream of access control.** (Reviewers: grep for the flag; if it appears in a worker or a payment module, that's a finding.) + +## Running a production test + +1. Mark participating accounts as testers. +2. Create the entity with a **small** stake, tick tester-only, schedule/publish it. +3. Preflight (funding, config) runs at creation/scheduling time, not at payout time — an underfunded run is refused before it exists, so you never drain a treasury to test the failure path. +4. **Verify results at the source of truth** — on-chain balance delta = prizes + gas, the third-party's own ledger, the recorded webhook body — not from your UI. The UI is one of the things under test. +5. Write up what the run proved and what it surfaced, with tx hashes, in the issue that tracked the launch. Numbers from the run become pinned tests. + +## Known gap to decide up front: aggregates + +Every app has some surface that sums over activity — a leaderboard, "total volume", analytics, a public counter. Tester activity flows into it unless excluded. Small volumes make it tolerable *today*, but it is an unexamined edge, not a decision. Decide per aggregate whether tester rows are included, record it in the repo's decisions doc, and pin the choice in a test. It is easier to exclude tester rows on day one than to explain a test artifact on a public board later. If aggregate standing ever gates prizes or promotion, exclusion becomes mandatory. + +## Porting checklist (use in the PR that introduces it) + +- [ ] Two DB flags (account capability, entity audience), admin-togglable, no deploy +- [ ] Every surface in the table above enumerated for this app and gated server-side; listing filters in the query +- [ ] 404 for hidden entities on direct lookup +- [ ] Re-check on in-flight reads and realtime subscribe +- [ ] `grep -r ` shows nothing downstream of access control +- [ ] Aggregate decision recorded and pinned in a test +- [ ] Preflight at creation time; smallest viable stake documented +- [ ] Verification recipe written down: where the source of truth is and what delta to expect +- [ ] Doc's update triggers listed (new public surface, flag-semantics change, aggregate decision) + +## Worked example: mini-quiz + +`User.isTester` / `Quiz.testerOnly` in `apps/api/prisma/schema.prisma`; composite index `[kind, testerOnly, status, scheduledStart]`. Enforced in `routes/quizzes.public.ts` (listing filter, by-code → 404), `routes/rooms.ts` (404 not 403), `services/room.service.ts` (`joinRoom` → `TESTER_ONLY`; lobby/submit/results re-checked per call), `routes/room-events.ts` (SSE subscribe). Viewer identity via `services/tester-access.service.ts`. Downstream (scheduler, scoring, payout worker, treasury) flag-blind. Known gap: `services/leaderboard.service.ts` and admin analytics aggregate tester XP/answers — acceptable while testers are internal and volumes tiny; decide per-aggregate if standing ever gates prizes. Update triggers: new public quiz surface (route, SSE channel, aggregate); change to flag semantics; the aggregate decision. diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml new file mode 100644 index 000000000..6a65ee5b3 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -0,0 +1,79 @@ +name: Bug report +description: Something is broken or behaves unexpectedly +title: "bug: " +labels: ["bug", "status: triage"] +body: + - type: textarea + id: what-happened + attributes: + label: What happened? (exact commands/steps + REAL output) + description: '"Confirmed" means you ran it. Distinguish "I ran this and here is the output" from "static reading suggests". Include what you expected instead.' + placeholder: "When I ... , the app ... . I expected ... ." + validations: + required: true + - type: textarea + id: repro + attributes: + label: Steps to reproduce + description: Numbered steps. An agent or teammate should be able to follow them exactly. + placeholder: | + 1. Go to ... + 2. Click ... + 3. See error + validations: + required: true + - type: textarea + id: rootcause + attributes: + label: Root cause (file:line), if known + description: Makes the fix unambiguous and the review checkable. "Static reading suggests" is fine — say which it is. + validations: + required: false + - type: input + id: version + attributes: + label: Version / commit tested + placeholder: "v2.4.13 / a1b2c3d, MiniPay Android" + validations: + required: false + - type: textarea + id: where + attributes: + label: "Where is the defect (not just where you noticed it)?" + description: Name the code path that serves users. If you found it in a fallback/dev-only path, check whether the production path has the same defect and say so. + placeholder: "Found in the log-scan route, but the subgraph path (production) has the identical fee omission." + validations: + required: false + - type: textarea + id: measurement + attributes: + label: "How would we know it's fixed?" + description: "What metric, log, or check would show the fix worked — without asking the reporter to retest. If the answer is 'we can't measure it', say so; instrumenting may be the first task." + validations: + required: false + - type: input + id: environment + attributes: + label: Environment + description: Browser/device, app version or URL, wallet if relevant (e.g. MiniPay). + placeholder: "MiniPay on Android 14, mondeto.app production" + validations: + required: false + - type: dropdown + id: severity + attributes: + label: Priority (one per issue — split if two items would be scheduled differently) + options: + - "critical — money correctness, security, or user-visible wrong data" + - "high — major feature broken, workaround exists" + - "medium" + - "low — minor / cosmetic" + validations: + required: true + - type: textarea + id: evidence + attributes: + label: Logs / screenshots + description: Console output, error messages, screenshots, tx hashes. + validations: + required: false diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 000000000..5316802eb --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: false +contact_links: + - name: Question / discussion + url: https://github.com/celo-org/mondeto/discussions + about: For open questions that aren't a bug or a planned piece of work. diff --git a/.github/ISSUE_TEMPLATE/task.yml b/.github/ISSUE_TEMPLATE/task.yml new file mode 100644 index 000000000..f17880c8c --- /dev/null +++ b/.github/ISSUE_TEMPLATE/task.yml @@ -0,0 +1,28 @@ +name: Task / chore +description: Technical or process work that isn't a user story or a bug (refactor, deps, infra, docs) +title: "task: " +labels: ["chore", "status: triage"] +body: + - type: textarea + id: what + attributes: + label: What needs to be done? + description: Concrete outcome, not activity. "CI runs tests on every PR" not "look into CI". + validations: + required: true + - type: textarea + id: done + attributes: + label: Definition of done + placeholder: | + - [ ] ... + - [ ] ... + validations: + required: true + - type: dropdown + id: priority + attributes: + label: Priority + options: ["critical — money correctness, security, or user-visible wrong data", "high", "medium", "low"] + validations: + required: true diff --git a/.github/ISSUE_TEMPLATE/user_story.yml b/.github/ISSUE_TEMPLATE/user_story.yml new file mode 100644 index 000000000..e29630298 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/user_story.yml @@ -0,0 +1,60 @@ +name: User story / feature +description: A piece of user-facing work, written so a human or agent can implement it without follow-up questions +title: "story: " +labels: ["enhancement", "status: triage"] +body: + - type: textarea + id: story + attributes: + label: User story + description: Who wants it, what they want, and why. + placeholder: "As a , I want , so that ." + validations: + required: true + - type: textarea + id: context + attributes: + label: Context & evidence + description: Why now? Link research, feedback, metrics, or the parent epic. + validations: + required: false + - type: textarea + id: acceptance + attributes: + label: Acceptance criteria + description: Verifiable checklist — these become the tests AND define "done" (a ticket closes when these are met, not when a related PR merges). Keep code work and ops work (rotate/migrate/purge) as separate boxes so a PR can close its half. + placeholder: | + - [ ] Given ..., when ..., then ... + - [ ] Given ..., when ..., then ... + - [ ] Errors are handled: ... + validations: + required: true + - type: textarea + id: out-of-scope + attributes: + label: Non-goals + description: What this deliberately does NOT include, and who owns the adjacent work. ("User-facing copy belongs to #193.") Also say if this is explicitly NOT a blocker. + validations: + required: true + - type: textarea + id: dependencies + attributes: + label: Depends on / blocks + description: Ordering that must survive without anyone remembering it. + placeholder: "Depends on #215. Blocks #216." + validations: + required: false + - type: dropdown + id: priority + attributes: + label: Priority + options: ["critical — money correctness, security, or user-visible wrong data", "high", "medium", "low"] + validations: + required: true + - type: textarea + id: notes + attributes: + label: Design / technical notes + description: Links to designs, affected files or modules, API changes, migration needs. + validations: + required: false diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..d39f0d23f --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,77 @@ + + +## The hole, and the fix + + + +## What this does NOT do / residual risk + + + +## Judgement calls + + + +## Issues + + +Closes # +Refs # + +## Stacking / conflicts + + + +## Verification evidence + + + +## Remaining ops steps + + +- [ ] none + +## Checklist + +- [ ] Title is the commit message I want on `main` +- [ ] Ships the test that fails on pre-fix code; fixtures are real captured data; fakes honour their arguments +- [ ] Covers the seam, not only pure functions; any stated guarantee tested on its failure path +- [ ] Error responses audited for what they leak; reads on write paths bounded at the query +- [ ] Judgement calls / bundled product changes flagged above (or "none") +- [ ] `lint` / `typecheck` / `test` pass locally on the current head +- [ ] Re-read acceptance criteria of every `Closes` issue — all met +- [ ] Development sidebar links match Closes/Refs above +- [ ] README / runbook / `.env.example` / examples / error strings updated for the world this creates +- [ ] Lockfile touched → rebased on current `main`, lockfile regenerated (never hand-resolved) +- [ ] Wallet/provider tree touched → loaded in a normal browser, not only MiniPay +- [ ] Money/security path → `money-path-checklist.md` run; payout logic compared against the other side +- [ ] No secrets in the diff +- [ ] Questions for the maintainer marked clearly at the end (or "none") + + diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..6d6409e10 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,58 @@ +# CLAUDE.md — docs + + + +## What this project is + +The official Celo documentation site (docs.celo.org), built with Mintlify. MDX pages plus a +`docs.json` config drive the site; developers building on Celo are the audience. There is no +application code, no package.json, and no unit tests — the product is the rendered docs. + +## Commands + +- Install the CLI (once): `npm i -g mint` +- Preview locally: `mint dev` — serves at `http://localhost:3000`; run from the repo root (needs `docs.json`) +- Check links: `mint broken-links` — run after EVERY change; this is what CI gates on +- CLI acting up: `mint update` +- CI: `.github/workflows/docs-validation.yml` runs `npx mintlify broken-links` on PRs — the required check is named `Check for broken links` + +## Architecture pointers + +- `docs.json` — Mintlify config: theme, and ALL navigation. A new page is invisible until added to a `navigation` tab/group here. +- Content dirs map 1:1 to nav tabs: `home/`, `build-on-celo/`, `tooling/`, `contribute-to-celo/`, `infra-partners/`, `specs/`, `legacy/`. +- `snippets/` — reusable MDX includes; `assets/`, `images/`, `img/` — static files. +- `_deprecated/` — retired pages kept out of nav; `submodules/` — externally sourced content. +- `scripts/update_contracts.py` — regenerates contract-address reference data. + +## Team rules (shared, synced — read them) + +@.claude/shared/engineering-rules.md + +The ten you must never violate, even without reading the above: +1. Never push to `main`. Branch `/-` → PR → squash. Title = the commit on `main` (Conventional Commits, scoped, outcome). +2. One concern per PR, one fix-unit per issue, one priority per issue. +3. Every change ships the test that fails on pre-fix code, through the seam it touches (route/CLI/component), and any prose guarantee is tested on its failure path. State the mutation count in the PR. +4. `Closes #N` only if every acceptance box is met; otherwise `Refs #N`. After merge, check what actually closed. +5. Every claim in an issue/PR/review is evidence-backed: commands + output, `file:line`. "Confirmed" means you ran it. Measured over reasoned — thresholds and constants pinned in a test, not a comment. +6. Say what the PR does NOT do (with numbers). Say what it actually does, even beyond the ticket. Flag judgement calls and bundled product changes for the maintainer. +7. On review feedback: reproduce first, fix, audit your own fix, report what it taught. Answer every point FIXED / NOT-FIXED / DISAGREE-with-measurement; never a silent push; never delete a wrong claim — strike it through. +8. Use only existing labels: `bug` `enhancement` `chore` `priority:critical|high|medium|low` `status: triage`. Never invent labels. +9. Ask before anything outward-facing or irreversible (external repos, posting, deleting, force-pushing shared branches). Propose, never execute. +10. No secrets in diffs. New env vars → `.env.example` + runbook, in the same PR. + +Use the plugin commands: `/file-issue`, `/write-pr`, `/review-pr`, `/post-merge`, `/close-pr`. + +## Product context + +- Live site: https://docs.celo.org +- Mintlify docs (authoring reference): https://mintlify.com/docs +- Community: https://discord.com/invite/celo + +## Gotchas + +- Adding an `.mdx` file is not enough — it 404s until listed in `docs.json` navigation. +- "Verification" here means `mint broken-links` green plus the changed pages rendering in `mint dev`; there is no test suite. +- Frontmatter `title` drives the sidebar label; keep it in every page. +- `submodules/` content is synced from other repos — don't hand-edit it here. +- Prefer relative internal links (`/build-on-celo/...`) over absolute `https://docs.celo.org/...` so the link checker can validate them. From 8aa2ac04434a27194fe24146757a261848a0f755 Mon Sep 17 00:00:00 2001 From: GigaHierz Date: Mon, 17 Aug 2026 23:26:00 +0100 Subject: [PATCH 2/3] docs: remove source/incident citations from shared rules (synced from pm-kit) Co-Authored-By: Claude Fable 5 --- .claude/shared/engineering-rules.md | 132 +++++++++++++------------ .claude/shared/money-path-checklist.md | 36 +++---- .claude/shared/tester-mode-pattern.md | 2 +- .github/ISSUE_TEMPLATE/user_story.yml | 4 +- 4 files changed, 88 insertions(+), 86 deletions(-) diff --git a/.claude/shared/engineering-rules.md b/.claude/shared/engineering-rules.md index cad5e9bf9..c0c95c8e1 100644 --- a/.claude/shared/engineering-rules.md +++ b/.claude/shared/engineering-rules.md @@ -2,131 +2,131 @@ One document, synced to every repo from `pm-kit`. Do not edit here — edit in `pm-kit/templates/.claude/shared/` and the sync opens PRs. -Merged from four sources: the Mondeto team guide (merges/tickets/closing), the celo-composer overhaul playbook (30+ issues → 30+ PRs in 4 days), the hardening playbook (PRs #49–#82, security/money paths), and the agent-assisted-development standards (recent PR/review rounds — cited `(agent-std)`). Every rule has an incident behind it — cited as `(mondeto #N)`, `(composer #N)`, `(hardening #N)`. Where sources disagreed, the resolution and reasoning is in §9. +Every rule here was earned by a real incident in our own products. The failure mechanisms are kept — they're what make the rules recognisable in the moment — but they are illustrations, not rankings: a rule without an example is just as binding as one with. -**One-paragraph philosophy (agent-std):** treat the agent's output as claims to be verified, not results to be trusted. Measure instead of reason wherever possible, and pin the measurement in a test. State what the change does *not* do as clearly as what it does. When a guarantee appears in prose, a test must prove it on every path — narrative must never outrun code. When a reviewer pushes back, reproduce their finding before touching anything. The bugs that survive live at the seams (wiring, error paths, bounds), so verification means running the artifact, not just its units. +**One-paragraph philosophy:** treat the agent's output as claims to be verified, not results to be trusted. Measure instead of reason wherever possible, and pin the measurement in a test. State what the change does *not* do as clearly as what it does. When a guarantee appears in prose, a test must prove it on every path — narrative must never outrun code. When a reviewer pushes back, reproduce their finding before touching anything. The bugs that survive live at the seams (wiring, error paths, bounds), so verification means running the artifact, not just its units. The two rules that hold everything else together: -1. **Evidence over assertion, at every stage.** Issues, PR bodies, reviews, and status comments are all claims. The only claims that survive unchanged are the ones somebody executed. State the *scope of your evidence*: "true of what I ran" is not "true of what a user gets" (composer). -2. **The record gets corrected, publicly, by whoever finds the error — including in their own work.** One uncorrected sentence in composer #423 caused three downstream fixes. Corrections are cheap; propagation is not. +1. **Evidence over assertion, at every stage.** Issues, PR bodies, reviews, and status comments are all claims. The only claims that survive unchanged are the ones somebody executed. State the *scope of your evidence*: "true of what I ran" is not "true of what a user gets". +2. **The record gets corrected, publicly, by whoever finds the error — including in their own work.** A single uncorrected sentence in an audit has caused multiple downstream fixes. Corrections are cheap; propagation is not. --- ## 1. Issues -**Verify before filing — "confirmed" means you ran it.** Distinguish "I ran this, here is the output" from "static reading suggests". Three exploration claims in the composer audit were wrong (a helper that "never returns true" worked; a "type-check failure" didn't exist; two "unwired" providers were wired) and one shipped into #423 and two docs PRs. Facts about the outside world get checked at the source: token addresses via `eth_call` not memory, package APIs against the installed package, advisories against the advisory API. Composer #388 contained fabricated contract addresses with correct-looking prefixes. +**Verify before filing — "confirmed" means you ran it.** Distinguish "I ran this, here is the output" from "static reading suggests". Audits routinely produce wrong static-reading claims (a helper that "never returns true" worked; a "type-check failure" didn't exist; "unwired" providers were wired) — and unverified ones ship into fixes and docs. Facts about the outside world get checked at the source: token addresses via `eth_call` not memory, package APIs against the installed package, advisories against the advisory API. Fabricated contract addresses with correct-looking prefixes have made it into filed issues. -**Name the defect in the code path that serves users, not where you noticed it.** Mondeto #182 reported a fee bug in the log-scan route, which only runs when the subgraph is unconfigured — never in production. The production path had the identical defect. Ask: is the location I found it the location users hit? +**Name the defect in the code path that serves users, not where you noticed it.** A fee bug was once reported in a fallback route that never runs in production — while the production path had the identical defect. Ask: is the location I found it the location users hit? -**One fix-unit per issue, one priority per issue.** Cluster by *fix boundary*, not observation boundary: one ticket per thing one person fixes in one pass over one file-set, with a checklist inside. **Same-diff test:** if two tickets would be closed by the same diff, merge them (composer #416: one ticket, nine locations, one fix — right shape; four docs tickets closed by one PR — wrong shape). **Different-schedule test:** if two items in one issue would be scheduled or prioritised differently, split them — bundling a live defect with low-priority hardening makes the defect inherit the low priority (hardening #47 → #81). **One symptom is not one defect:** mondeto #196 conflated a blank map (parse failure) with a freeze (main-thread block); work on one looked like progress on the other. It became #225 + #196 + #226. AI makes finding cheap — findings outnumber fix-units ~4:1 — so cluster before filing, never map finding → ticket 1:1. +**One fix-unit per issue, one priority per issue.** Cluster by *fix boundary*, not observation boundary: one ticket per thing one person fixes in one pass over one file-set, with a checklist inside. **Same-diff test:** if two tickets would be closed by the same diff, merge them (one ticket covering nine locations fixed by one diff is the right shape; four tickets closed by one PR is the wrong shape). **Different-schedule test:** if two items in one issue would be scheduled or prioritised differently, split them — bundling a live defect with low-priority hardening makes the defect inherit the low priority. **One symptom is not one defect:** a blank screen (parse failure) and a freeze (main-thread block) were once conflated in one ticket; work on one looked like progress on the other until they were split. AI makes finding cheap — findings outnumber fix-units roughly 4:1 — so cluster before filing, never map finding → ticket 1:1. -**Anatomy of a good issue:** what happens (exact commands + real output, or user steps + observed) → root cause (`file:line`) → impact (who hits this doing what; for security/money, what the attacker gets — hardening #54 said "quota theft, a DoS lever against settlement, lands in rotated container logs and pasted debug output", which is what got it prioritised) → suggested fix → version/commit tested. Use the issue forms in `.github/ISSUE_TEMPLATE/`. +**Anatomy of a good issue:** what happens (exact commands + real output, or user steps + observed) → root cause (`file:line`) → impact (who hits this doing what; for security/money, what the attacker gets — "quota theft, a DoS lever against settlement, lands in rotated container logs" is the sentence that gets an issue prioritised) → suggested fix → version/commit tested. Use the issue forms in `.github/ISSUE_TEMPLATE/`. -**State non-goals, dependencies, and what is explicitly not a blocker.** Mondeto #201's single sentence "Explicitly not a blocker" kept it out of two sprint discussions. #215/#193 both name who owns the shared file. #216 says "Depends on #215" in the body so the ordering survives without memory. +**State non-goals, dependencies, and what is explicitly not a blocker.** A single sentence "Explicitly not a blocker" keeps an issue out of sprint discussions. Name who owns a shared file; put "Depends on #N" in the body so ordering survives without memory. -**Acceptance criteria as checkboxes; separate code work from ops work.** Hardening #54 lists "rotate the credential / recreate the container / purge captures" as their own boxes, so a code PR can close its half while the ops half stays tracked (#72: "this stops the repo adding to the exposure; rotation actually closes it"). +**Acceptance criteria as checkboxes; separate code work from ops work.** List "rotate the credential / recreate the container / purge captures" as their own boxes, so a code PR can close its half while the ops half stays tracked ("this stops the repo adding to the exposure; rotation actually closes it"). -**Say how we'd know it's fixed.** Before asking a reporter to retest, have a way to answer yourself. Mondeto #196 couldn't be measured — a client that fails to parse never fires analytics — so two support reports were the entire evidence base. Instrument first (#226), then ask, or better, just look. +**Say how we'd know it's fixed.** Before asking a reporter to retest, have a way to answer yourself. A client that fails to parse never fires analytics — so support reports were once the entire evidence base for a bug. Instrument first, then ask, or better, just look. -**Record negative results precisely or not at all.** Composer draft #204's "tried and did not work" table said `transpilePackages` doesn't work; it does — it was aimed at packages that don't publish the failing code. Record *why* it failed precisely enough to tell "doesn't work" from "applied wrong". When unsure, write *attempted, outcome unclear*, never *ruled out*. +**Record negative results precisely or not at all.** A "tried and did not work" table once ruled out an approach that works — it had been aimed at the wrong target. Record *why* it failed precisely enough to tell "doesn't work" from "applied wrong". When unsure, write *attempted, outcome unclear*, never *ruled out*. -**Write the defect as a mechanism, with measurements** ("225 of 574 responses were template output") — numbers in the issue become pinned tests in the fix (agent-std). **Mark open questions as open** ("number left to implementer") so the implementer surfaces the judgement call instead of guessing silently. **Pin prior art to a commit SHA**, not a branch — branch links rot. Acceptance checklists include documentation requirements so they can't be dropped silently. For audits, split into lettered fixes (A–G) mapping 1:1 to PRs, with a status table on the parent issue. A root cause found during unrelated work (a CI flake, an infra gap) gets its own issue "so it outlives the PR". +**Write the defect as a mechanism, with measurements** ("225 of 574 responses were template output") — numbers in the issue become pinned tests in the fix. **Mark open questions as open** ("number left to implementer") so the implementer surfaces the judgement call instead of guessing silently. **Pin prior art to a commit SHA**, not a branch — branch links rot. Acceptance checklists include documentation requirements so they can't be dropped silently. For audits, split into lettered fixes (A–G) mapping 1:1 to PRs, with a status table on the parent issue. A root cause found during unrelated work (a CI flake, an infra gap) gets its own issue so it outlives the PR. -**When your issue turns out wrong, correct it in-thread:** what was wrong, what's true, and who downstream inherited it (composer #423 named the two PRs). Retitle when scope narrows (composer #463). One home per work item. +**When your issue turns out wrong, correct it in-thread:** what was wrong, what's true, and who downstream inherited it — name the affected PRs. Retitle when scope narrows. One home per work item. -**Priority lives in labels** — `priority:critical|high|medium|low` — not board fields (the shared DevRel board's Priority field has zero options; it is impossible to mark anything high there). `priority:critical` = money correctness, security, or user-visible wrong data. Don't inflate it. Add every new issue to the board when filed, not in batches. +**Priority lives in labels** — `priority:critical|high|medium|low` — not board fields. `priority:critical` = money correctness, security, or user-visible wrong data. Don't inflate it. Add every new issue to the board when filed, not in batches. --- ## 2. Pull requests -**Branch → PR → squash merge into `main`. Nothing goes straight to `main`.** Branch names: `/-`, slug naming the *problem, not your solution* (`lena/196-minipay-freeze`); bots keep their prefix (`renovate/...`). (Resolution of a three-way disagreement — see §9.) +**Branch → PR → squash merge into `main`. Nothing goes straight to `main`.** Branch names: `/-`, slug naming the *problem, not your solution* (`lena/196-minipay-freeze`); bots keep their prefix (`renovate/...`). (A close call — see §9.) **The PR title is the commit on `main`** (squash with PR title). Conventional Commits, scoped, imperative, stating the *outcome*: `fix(buy): block buying ocean pixels via long-press inspect path`, not `fix buy bug`. -**Smallest correct diff, one concern per PR.** Composer #427's two-line quote fix unblocked six PRs. Big mixed PRs stall behind their weakest part. If it builds on another PR, say so in the first line, name the base, give review order, offer to rebase (hardening #76: "Stacked on #75. Merge #75 first and this retargets cleanly."). Independent PRs say "branched off main, independent of my other open PRs" (#79). Keep stacks shallow (depth ≤ 2); with squash merges the second-lander rebases with `git rebase --onto main ` after the base merges. +**Smallest correct diff, one concern per PR.** Big mixed PRs stall behind their weakest part; a two-line fix that unblocks a queue of stacked PRs beats a bundled cleanup. If it builds on another PR, say so in the first line, name the base, give review order, offer to rebase ("Stacked on the base PR. Merge that first and this retargets cleanly."). Independent PRs say "branched off main, independent of my other open PRs". Keep stacks shallow (depth ≤ 2); with squash merges the second-lander rebases with `git rebase --onto main ` after the base merges. -**Every sentence in the body is a claim that will be adversarially checked.** Composer bodies that overclaimed all got caught ("only one place interpolates the name" — a second in JS; "the suite would have caught #399" — its check was a no-op). Write bodies you'd bet on, with **verification evidence as commands + output**, not "tested ✓". Include the **mutation count** (§3). +**Every sentence in the body is a claim that will be adversarially checked.** Bodies that overclaim get caught ("only one place interpolates the name" — there was a second; "the suite would have caught this" — its check was a no-op). Write bodies you'd bet on, with **verification evidence as commands + output**, not "tested ✓". Include the **mutation count** (§3). -**Surface judgement calls the spec left open as explicit, reversible decisions** ("named constant, one-line change if you want it more sensitive"). Never smuggle product decisions into bug fixes; if one is bundled, flag it for the maintainer to keep or drop ("happy to drop it; the fix stands without it"). Push back on the spec when the evidence disagrees, *with the evidence* — correcting an issue's overstatement is a contribution (agent-std). +**Surface judgement calls the spec left open as explicit, reversible decisions** ("named constant, one-line change if you want it more sensitive"). Never smuggle product decisions into bug fixes; if one is bundled, flag it for the maintainer to keep or drop ("happy to drop it; the fix stands without it"). Push back on the spec when the evidence disagrees, *with the evidence* — correcting an issue's overstatement is a contribution. -**Say what the PR actually does, and what it does NOT do — with numbers where possible** ("blocks repeats, not first submissions — roughly 190 of the 225"). Known accepted limitations are pinned in a test, not left implicit, so a future change has to confront them. Mondeto #224 said "data layer only" and also retired a product tab, rewired `/ranks`, and rewrote FAQ answers — reviewed under a label that said it wasn't there. Hardening #75's "what this does not buy" section ("whoever controls the endpoint completely can forge the receipt too; the independent second source is the follow-up, and this is not it") prevents a phrase like "verified on chain" from stopping people asking questions. +**Say what the PR actually does, and what it does NOT do — with numbers where possible** ("blocks repeats, not first submissions — roughly 190 of the 225"). Known accepted limitations are pinned in a test, not left implicit, so a future change has to confront them. A PR labelled "data layer only" once also retired a product tab, rewired a route, and rewrote FAQ answers — and was reviewed under a label that said it wasn't there. A "what this does not buy" section ("whoever controls the endpoint completely can forge the receipt too; the independent second source is the follow-up, and this is not it") prevents a phrase like "verified on chain" from stopping people asking questions. -**`Closes #N` is a contract: only when the entire issue's acceptance criteria are met. Otherwise `Refs #N`** and list which boxes this closes. Composer edited #448/#454 mid-flight for this; mondeto #209 got it right in prose and was overridden anyway (§6). +**`Closes #N` is a contract: only when the entire issue's acceptance criteria are met. Otherwise `Refs #N`** and list which boxes this closes. -**Never delete a wrong claim from a PR body — strike it through with a pointer to the correcting commit.** The visible correction is the trust signal (agent-std). Because we squash-merge with PR title + body, the PR body *is* the commit message on `main`: rationale, verification performed, known limits, and `Closes`/`Refs` trailers all live there. Keep agent co-author trailers; they're honest and cost nothing. +**Never delete a wrong claim from a PR body — strike it through with a pointer to the correcting commit.** The visible correction is the trust signal. Because we squash-merge with PR title + body, the PR body *is* the commit message on `main`: rationale, verification performed, known limits, and `Closes`/`Refs` trailers all live there. Keep agent co-author trailers; they're honest and cost nothing. **For a stack, verify the merged union:** merge all open branches into one tree locally and run the suite — each PR green individually says nothing about the combination. **When you touch CI, prove it in both directions:** link one green run and one deliberately red run. -**Reproduce before you fix, fix the root cause, collapse duplicated facts.** Hardening #77 wrote both attacks as tests, ran them red on main, then green. Composer's wallet-provider mess existed because one fact lived in four places and the copies drifted; #461 moved it to one module. If your fix restates a fact somewhere, ask where else it lives. State the trade-off that picked the design — "a missing file fails loudly at once, a missing variable misbehaves silently at runtime" (hardening #74) is the strongest one-line justification a PR can carry. +**Reproduce before you fix, fix the root cause, collapse duplicated facts.** Write the attack as a test, run it red on main, then green on the branch. Duplication is where drift starts: when one fact lives in four places the copies diverge — move it to one module, and if your fix restates a fact somewhere, ask where else it lives. State the trade-off that picked the design — "a missing file fails loudly at once, a missing variable misbehaves silently at runtime" is the strongest one-line justification a PR can carry. -**Flag breaking internal changes for parallel work** (hardening #77's signature change broke #79 — the note made it expected). **Know your conflicts before the reviewer does:** when PRs touch shared files, say so, propose a merge order, pre-write the resolution hunk (composer #428/#450). Whoever merges second rebases. +**Flag breaking internal changes for parallel work** — a signature change that breaks a sibling PR is fine when announced, a surprise when not. **Know your conflicts before the reviewer does:** when PRs touch shared files, say so, propose a merge order, pre-write the resolution hunk. Whoever merges second rebases. -**Runbooks, README, `.env.example`, examples, and error strings are part of the diff.** Grep for the old instructions (hardening #74's README still pointed at the old secret layout; stale docs "asserting the property an issue exists to create are worse than silence, because they stop anyone checking" — #76). Runbook steps get reviewed like code (#74's was missing `--build`). +**Runbooks, README, `.env.example`, examples, and error strings are part of the diff.** Grep for the old instructions — stale docs asserting the property an issue exists to create are worse than silence, because they stop anyone checking. Runbook steps get reviewed like code. -**Ask before public or irreversible actions** — filing on external repos, posting from the org, force-pushing shared branches, deleting branches, anything outward-facing. Track the decision as an owned issue (hardening #80), not a PR thread. **This applies doubly to agents: propose, never execute, anything outward-facing.** +**Ask before public or irreversible actions** — filing on external repos, posting from the org, force-pushing shared branches, deleting branches, anything outward-facing. Track the decision as an owned issue, not a PR thread. **This applies doubly to agents: propose, never execute, anything outward-facing.** -**Lockfile PRs — the rule that burned Mondeto twice (#162, #212):** never merge two lockfile-touching PRs without rebasing the second. Merge one → wait for `main` to update → rebase the next and **regenerate the lockfile, never hand-resolve** → wait for green on the rebased head → merge. Renovate only rebases when it next runs. (The ruleset's "require branch up to date" now enforces the rebase mechanically; the regenerate-don't-hand-resolve part is still yours.) +**Lockfile PRs — a rule that has burned us twice:** never merge two lockfile-touching PRs without rebasing the second. Merge one → wait for `main` to update → rebase the next and **regenerate the lockfile, never hand-resolve** → wait for green on the rebased head → merge. Renovate only rebases when it next runs. (The ruleset's "require branch up to date" now enforces the rebase mechanically; the regenerate-don't-hand-resolve part is still yours.) --- ## 3. Tests & verification -**Every change ships with the test that would have caught the bug** — the assertion that fails on pre-fix code, not merely a test that passes. Composer #461 v1 broke every MiniPay scaffold while its tests stayed green because the fixture asserted components existed but not that the manifest kept their dependencies. +**Every change ships with the test that would have caught the bug** — the assertion that fails on pre-fix code, not merely a test that passes. A fix once broke every generated scaffold while its tests stayed green, because the fixture asserted components existed but not that the manifest kept their dependencies. -**Cover the seams, not just the units.** Pure-function tests are necessary, not sufficient — the bugs that survive live at the wiring: response bodies, query bounds, component props, error paths, config parsing. Every PR has at least one test *through* the seam it touches (route in → response out; CLI as subprocess; component render) (agent-std). **A stated guarantee needs a test on every path** — if prose says "the error response never reveals X", a test asserts it on the *rejected* path, not just the accepted one. **Measured over reasoned:** any threshold, constant, or "this can't happen" is justified by a measurement pinned in a test, never only in a comment. Edge classes to check every time: Unicode/non-Latin input; empty input; the band between two thresholds (clears one floor, misses another); exact boundary values (`>=` vs `>`); concurrent duplicate requests. +**Cover the seams, not just the units.** Pure-function tests are necessary, not sufficient — the bugs that survive live at the wiring: response bodies, query bounds, component props, error paths, config parsing. Every PR has at least one test *through* the seam it touches (route in → response out; CLI as subprocess; component render). **A stated guarantee needs a test on every path** — if prose says "the error response never reveals X", a test asserts it on the *rejected* path, not just the accepted one. **Measured over reasoned:** any threshold, constant, or "this can't happen" is justified by a measurement pinned in a test, never only in a comment. Edge classes to check every time: Unicode/non-Latin input; empty input; the band between two thresholds (clears one floor, misses another); exact boundary values (`>=` vs `>`); concurrent duplicate requests. **Error paths are an attack surface.** Audit what every 4xx/5xx body leaks — exact scores are gradient oracles, flags are detection oracles. Decide per check whether it fails open or closed, write it down, verify the code matches. **Reads on write paths are bounded at the query** (`take(limit)`), never collect-then-slice — a fail-open catch around an unbounded read silently disables the check for exactly the heaviest users. **Config safety:** no silent fallbacks for critical config — unset or unrecognised values are errors, not defaults (a quiet default has shipped the wrong environment to production). Env values compared exactly — set with `printf`, never `echo` (stray newline). Build-time-inlined vars (`NEXT_PUBLIC_*`) need a fresh build; dashboard edits alone do nothing. Never print secret values — check presence, not content; keys never in argv, shell history, or committed files. -**Mutation-test the fix and state the count** ("disabling receipt verification turns 5 red" — hardening). A suite equally green with and without the fix proves nothing. +**Mutation-test the fix and state the count** ("disabling receipt verification turns 5 red"). A suite equally green with and without the fix proves nothing. -**Pair every absence-assertion with a control.** "Nothing bad happened" must run beside "something happened at all", or it passes against code that never ran (hardening #75 caught four vacuous passes this way). **Check that "never happened" assertions can fail** — hardening #82 asserted `settleCalls() === 0` on a path where the counter can't increment. Make it fail by hand first. +**Pair every absence-assertion with a control.** "Nothing bad happened" must run beside "something happened at all", or it passes against code that never ran — one review caught four vacuous passes this way. **Check that "never happened" assertions can fail** — a `calls === 0` assertion on a path where the counter can't increment proves nothing. Make it fail by hand first. -**Fixtures are real captured data.** Mondeto #220's classifier tests asserted on `'execution reverted'` — a bare fragment that never occurs; real viem errors carry a `Docs: https://viem.sh/...` line, which the rule matched on `http`, so real reverts were filed as network errors. Green tests, wrong behaviour. +**Fixtures are real captured data.** Classifier tests once asserted on the bare fragment `'execution reverted'`, which never occurs in real errors — the real ones carry a docs URL that a different rule matched first, so real reverts were misfiled as network errors. Green tests, wrong behaviour. -**Test doubles honour their inputs.** A fake `getLogs` that ignores the filter it's handed silently turns every test depending on that filter into a vacuous one (hardening #76, #78). +**Test doubles honour their inputs.** A fake `getLogs` that ignores the filter it's handed silently turns every test depending on that filter into a vacuous one. -**Assert at the boundary where damage happens** — the recorded webhook POST body, not stdout, "because a leak that only reaches Slack never appears in the console" (hardening #72). **Verify deployment claims from inside the running system**, and test the existing-state case (populated DB, root-owned volume), not just fresh state (#74, #77). +**Assert at the boundary where damage happens** — the recorded webhook POST body, not stdout, because a leak that only reaches the messaging channel never appears in the console. **Verify deployment claims from inside the running system**, and test the existing-state case (populated DB, root-owned volume), not just fresh state. -**Paths that cannot be faked get run in production with a restricted audience — tester mode.** Staging proves code against a mock; it cannot prove real gas leaves the real treasury. Two DB flags (account capability + entity audience), server-side gating at every surface (listing, lookup → 404, join, in-flight reads, realtime, aggregates), and everything downstream flag-blind so the tester run is the identical pipeline with real (small) stakes. Mini-quiz's first native-CELO payout ran this way and surfaced three bugs (#19–#21) no offline test could reach. Verify at the source of truth, not the UI. Pattern and porting checklist: `tester-mode-pattern.md`. +**Paths that cannot be faked get run in production with a restricted audience — tester mode.** Staging proves code against a mock; it cannot prove real gas leaves the real treasury. Two DB flags (account capability + entity audience), server-side gating at every surface (listing, lookup → 404, join, in-flight reads, realtime, aggregates), and everything downstream flag-blind so the tester run is the identical pipeline with real (small) stakes. Our first real-money payout ran this way and surfaced three bugs no offline test could reach. Verify at the source of truth, not the UI. Pattern and porting checklist: `tester-mode-pattern.md`. -**Anything that decides who gets paid gets compared byte-for-byte against the other side** — contract, on-chain read, the other route (mondeto #48, #224 where the comparator was identical but the block window wasn't; #223 checked fee arithmetic against `feeRate()` live on all eight map contracts). Money/security diffs additionally run `money-path-checklist.md`. +**Anything that decides who gets paid gets compared byte-for-byte against the other side** — contract, on-chain read, the other route — including the *window* handed to an identical comparator (two comparators can match while their block windows don't). Check fee arithmetic against the live contract value, not a copy. Money/security diffs additionally run `money-path-checklist.md`. -**We only test MiniPay, so browser regressions go unnoticed.** Mondeto #221 broke `mondeto.app` in every browser for four days. Wallet/provider tree changes get loaded in a normal browser too. +**Test in a normal browser, not only the primary wallet client.** A wallet/provider-tree change once broke the public site in every browser for days while the in-wallet experience stayed fine. Wallet/provider tree changes get loaded in a normal browser too. -**A green check is only as good as what the check runs.** Composer's CI was secret-scanning only for days — every broken PR was green. A "TypeScript check" resolved to npm's placeholder package `tsc` and certified 21 syntax errors. Know what gates actually verify; the CI in `pm-kit` fails without lint/test scripts and uses `--no-install` for that reason. **A green check reflects the commit it ran on** — mondeto #206's green run predated the bump it needed to guard. +**A green check is only as good as what the check runs.** CI that is secret-scanning-only makes every broken PR green. A "TypeScript check" can resolve to npm's placeholder package named `tsc` and certify syntax errors. Know what gates actually verify; the CI in `pm-kit` fails without lint/test scripts and uses `--no-install` for exactly this reason. **A green check reflects the commit it ran on** — a green run that predates the change it needed to guard counts for nothing. --- ## 4. Reviewing -**Review by attempting to refute, with the code running.** Reading diffs catches style; running catches a fix that silently stripped a template's wallet stack, a tarball shipping the files a script existed to exclude, a `tsc` that wasn't TypeScript (composer). **Tier it — uniform max-rigour was the real cost driver in the composer overhaul:** +**Review by attempting to refute, with the code running.** Reading diffs catches style; running catches a fix that silently stripped a template's wallet stack, a tarball shipping the files a script existed to exclude, a type checker that wasn't one. **Tier it — uniform max-rigour is a real cost driver:** - *Mechanical* (renames, quoting, doc paths, green dep bumps): read + confirm what CI ran. No behaviour pass. - *Logic, new surface, funds/security/release, wallet tree*: full pass — run the suite on the branch and prove tests can go red; build the real artifact (for a scaffolder, the generated project *is* the product); click through the changed surface once (dev server or the CLI command a user would run). Optionally a cloud multi-agent review first as a breadth pass — treat its output as claims to verify, and it never replaces the behaviour pass. -**Verify claims against code, never against the description.** Hardening's descriptions were excellent and still wrong in places (#76 mutation table, #75 "nothing on chain could spoof this"). Half-wrong claims: identify the right half (composer's "both files wrong" — one was correct). **Verify the feedback itself before implementing it** — reviewers are claims too; two composer docs PRs implemented an incorrect issue premise that 30 seconds of grep would have caught. +**Verify claims against code, never against the description.** Excellent descriptions are still wrong in places. Half-wrong claims: identify the right half. **Verify the feedback itself before implementing it** — reviewers are claims too; incorrect issue premises have been implemented verbatim when thirty seconds of grep would have caught them. -**Review against current `main`, on the head SHA you think you're reviewing.** Branches go stale in a queue; `gh pr diff` served stale content twice. **When CI is red, first ask what moved** — hardening #73's runner had a floating Node version, so main went red with no commit and every open PR looked guilty. +**Review against current `main`, on the head SHA you think you're reviewing.** Branches go stale in a queue; `gh pr diff` can serve stale content. **When CI is red, first ask what moved** — a runner with a floating Node version once turned main red with no commit behind it, and every open PR looked guilty. -**A PR description is testimony, not evidence** — the review's job is to find where the narrative outran the code. **Recompute the math:** pinned constants, statistical formulas, thresholds — rederive independently before trusting them. Probe the standard edge classes and the fail-open/fail-closed direction of every guard; audit error responses for leaks; check read bounds on write paths. **Separate CI signal:** say which failures are code and which are infra/permissions. **Name what the PR got right** — a request-changes that ends "one round of targeted fixes, not a rethink" gets a better resolution round than a bare defect list; every finding includes the shape of the fix ("one line: strip the flag when block is true"). **For payment paths, auth, or anything moving money: adversarially verify findings** — a second independent pass attempting to refute each — before posting; plausible-but-wrong findings burn author trust and review rounds (agent-std). +**A PR description is testimony, not evidence** — the review's job is to find where the narrative outran the code. **Recompute the math:** pinned constants, statistical formulas, thresholds — rederive independently before trusting them. Probe the standard edge classes and the fail-open/fail-closed direction of every guard; audit error responses for leaks; check read bounds on write paths. **Separate CI signal:** say which failures are code and which are infra/permissions. **Name what the PR got right** — a request-changes that ends "one round of targeted fixes, not a rethink" gets a better resolution round than a bare defect list; every finding includes the shape of the fix ("one line: strip the flag when block is true"). **For payment paths, auth, or anything moving money: adversarially verify findings** — a second independent pass attempting to refute each — before posting; plausible-but-wrong findings burn author trust and review rounds. -**Independently confirm the highest-severity finding before acting** (both hardening HIGHs were re-verified line-by-line). A wrong HIGH burns trust; a confirmed one with `file:line` and an attack script is undeniable. +**Independently confirm the highest-severity finding before acting.** A wrong HIGH burns trust; a confirmed one with `file:line` and an attack script is undeniable. **Every finding: severity · `file:line` · one-sentence defect · concrete failure scenario · suggested fix.** "The rate limits interact badly" is not implementable; "both throttles build the key `ip:`, `take()` refills at the calling tier, so alternating requests degrades 10/hour to 60/min — namespace them" is a 20-minute fix. **Say what you verified as sound** — it tells the author what not to touch and proves you read the code. -**Verdicts:** `APPROVE` (zero open findings, however small) / `REQUEST-CHANGES` (anything else). There is no approve-with-nits: a PR is not mergeable while any finding is unresolved. A finding resolves as exactly one of: a fix commit on the PR, or — only when genuinely out of scope for the PR — a follow-up issue *filed and linked from the review*, never merely suggested. "Could be a new ticket" / "out of scope" in prose, with no issue behind it, leaves the finding open; deferred work always lands in a filed issue, otherwise it evaporates (composer #452, #460). **Review the tests as hard as the code:** if nothing realistic would make the suite go red, it's worse than none — it certifies broken output (composer #447 v1). +**Verdicts:** `APPROVE` (zero open findings, however small) / `REQUEST-CHANGES` (anything else). There is no approve-with-nits: a PR is not mergeable while any finding is unresolved. A finding resolves as exactly one of: a fix commit on the PR, or — only when genuinely out of scope for the PR — a follow-up issue *filed and linked from the review*, never merely suggested. "Could be a new ticket" / "out of scope" in prose, with no issue behind it, leaves the finding open; deferred work always lands in a filed issue, otherwise it evaporates. **Review the tests as hard as the code:** if nothing realistic would make the suite go red, it's worse than none — it certifies broken output. -**For a queue of PRs, build the conflict matrix mechanically** (`git merge-tree`) and publish the merge order; name the merge-order hazards (shared files, squash retargeting of stacks, signature changes) so the second merger expects the conflict. Sequenced merges kept every intermediate `main` working across ~30 composer merges. +**For a queue of PRs, build the conflict matrix mechanically** (`git merge-tree`) and publish the merge order; name the merge-order hazards (shared files, squash retargeting of stacks, signature changes) so the second merger expects the conflict. Sequenced merges can keep every intermediate `main` working across dozens of merges. --- ## 5. Receiving feedback -The review round is where contributor quality is actually measured. In order (agent-std): +The review round is where contributor quality is actually measured. In order: 1. **Reproduce before touching anything.** Run the reviewer's scenario and confirm it. If you can't reproduce, say so plainly — and still fix on the evidence if the mechanism is sound. Never implement a fix for a finding you haven't understood: a plausible fix for a misdiagnosed problem is two bugs. 2. **Fix, then audit your own fix** the way the reviewer reviewed the original. If the audit finds a bug in the fix, disclose it unprompted. A fix that silently changes scope is a new unreviewed change. @@ -134,13 +134,13 @@ The review round is where contributor quality is actually measured. In order (ag 4. **Push back only with a measurement** — an on-chain read, a header trace, a control run — not intuition. 5. **Defer ownership calls.** If a finding opens a design question that belongs to someone else, present options with trade-offs and ask; time-box it ("say what shape you want and I'll push it today"). Don't grow a tightly-scoped PR on your own judgement. -**Answer every review point explicitly: `FIXED` / `NOT-FIXED` / `DISAGREE-because`.** Responses that silently addressed some points forced full re-reviews of everything (composer). Every review ask ends as one of: a commit, a filed follow-up issue, or an explicit "won't do, because…" — never silence. +**Answer every review point explicitly: `FIXED` / `NOT-FIXED` / `DISAGREE-because`.** Responses that silently address some points force full re-reviews of everything. Every review ask ends as one of: a commit, a filed follow-up issue, or an explicit "won't do, because…" — never silence. -**Never respond with a silent push.** Push, then comment what changed and which findings it addresses (composer's early rounds were silent force-pushes; nobody could tell what moved). With squash merges you may rebase/force-push your own branch freely — the *comment* is the rule. +**Never respond with a silent push.** Push, then comment what changed and which findings it addresses — after a round of silent force-pushes nobody can tell what moved. With squash merges you may rebase/force-push your own branch freely — the *comment* is the rule. -**When review proves your fix wrong, the model response is composer #461's revision:** adopt the diagnosis, encode why in the code itself ("Caught in review on #461" now lives in a doc comment), add the missing assertion, say what you verified. Turnaround beat defensiveness by a day. **Add the test the reviewer says is missing, especially when it would have caught the finding** — each hardening HIGH came with the exact test gap that hid it. +**When review proves your fix wrong, the model response is:** adopt the diagnosis, encode why in the code itself (a "caught in review" doc comment at the site), add the missing assertion, say what you verified. Turnaround beats defensiveness. **Add the test the reviewer says is missing, especially when it would have caught the finding** — high-severity findings usually come with the exact test gap that hid them. -**Answer direct questions in PR threads promptly** — an unanswered go/no-go blocks the author invisibly (hardening #72). If it needs a decision process, convert to an owned issue and say so. +**Answer direct questions in PR threads promptly** — an unanswered go/no-go blocks the author invisibly. If it needs a decision process, convert to an owned issue and say so. --- @@ -148,13 +148,13 @@ The review round is where contributor quality is actually measured. In order (ag **Merge what's ready promptly** — an approved PR sitting unmerged makes every stacked PR staler. Keep stacks rebased after the base merges. Call out schema migrations in merge notes. -**Approvals don't survive a re-roll.** Mondeto #89 carried a sign-off for pnpm v10; Renovate re-rolled it to v11.5.3 with the approval still sitting there. (Ruleset dismisses stale approvals on push now — but a re-approval is a re-review, not a click.) +**Approvals don't survive a re-roll.** A dependency PR once carried a sign-off for one major version after the bot re-rolled it to the next. (The ruleset dismisses stale approvals on push now — but a re-approval is a re-review, not a click.) -**After merging, check what closed.** Mondeto #196 closed one second after #209 merged, even though #209's body said in bold it does not close #196 and its footer read `Closes #208. Refs #196.` — a Development-sidebar link outranks anything in the body. It stayed closed five days while the remaining work lived only in a draft PR. Rules: **sidebar links close issues; prose does not stop them** — unlink what you don't intend to close. **A ticket is done when its acceptance criteria are met, not when a related PR merges** — re-read them before closing. **If closing orphans remaining work, file the successor first** and link it from the closing comment. Run `/post-merge ` after every merge until it's a habit. +**After merging, check what closed.** An issue once closed the second a *related* PR merged — the PR's body said in bold it does not close that issue, and its footer said `Refs` — because a Development-sidebar link outranks anything in the body. It stayed wrongly closed for days. Rules: **sidebar links close issues; prose does not stop them** — unlink what you don't intend to close. **A ticket is done when its acceptance criteria are met, not when a related PR merges** — re-read them before closing. **If closing orphans remaining work, file the successor first** and link it from the closing comment. Run `/post-merge ` after every merge until it's a habit. -**Close issues with evidence, not "done":** name the fixing PR and what was verified. Close stale issues and supersede stale PRs with the same specificity — thank the contributor, cite the superseding commit, invite fresh work (one such contributor returned with a mergeable PR the next day). **Every open PR carries a current status:** merged, reviewed with verdict, or what it's waiting on. +**Close issues with evidence, not "done":** name the fixing PR and what was verified. Close stale issues and supersede stale PRs with the same specificity — thank the contributor, cite the superseding commit, invite fresh work (contributors treated this way come back with mergeable PRs). **Every open PR carries a current status:** merged, reviewed with verdict, or what it's waiting on. -**Closing a PR without merging — capture what it proved.** Unmerged work often contains proven pieces (a working flow, validated thresholds). Identify what's reusable, capture it in the issue that carries the work forward with file links pinned to the PR's head SHA, close with a comment saying why and where the value went, keep the branch unless there's a reason to delete (agent-std). **When CI goes red:** diagnose to root cause, prove it with a control run (no code change), file the diagnosis as an issue so it outlives the PR. Policy calls (retries, concurrency limits) go to the maintainer, flagged — not decided unilaterally. +**Closing a PR without merging — capture what it proved.** Unmerged work often contains proven pieces (a working flow, validated thresholds). Identify what's reusable, capture it in the issue that carries the work forward with file links pinned to the PR's head SHA, close with a comment saying why and where the value went, keep the branch unless there's a reason to delete. **When CI goes red:** diagnose to root cause, prove it with a control run (no code change), file the diagnosis as an issue so it outlives the PR. Policy calls (retries, concurrency limits) go to the maintainer, flagged — not decided unilaterally. **Versioning:** the version lives in the repo, release tooling reads it, CI verifies tag == `package.json` and never invents versions. Changelog entries land with the change. @@ -174,19 +174,21 @@ observe (run it) → file (claims + evidence) → fix (smallest diff + the test → merge (in published dependency order) → verify end-to-end on main → check what closed → correct the record ``` -Calibration: composer day 1, zero of six templates built; day 4, seven of seven build end-to-end with CI gating on a real scaffold suite. The delta was verification applied uniformly to everyone's claims, including our own. +Calibration, from one four-day overhaul: day 1, zero of six templates built; day 4, seven of seven built end-to-end with CI gating on a real scaffold suite. The delta was verification applied uniformly to everyone's claims, including our own. --- -## 9. Where the sources disagreed — and what we chose +## 9. Close calls — and what we chose -- **Branch naming** — three-way: kit v1 `feat/…`, Mondeto `/`, agent-std `type/issue-slug`. Chose `/-`: the type already lives in the Conventional-Commit PR title (so a type prefix is redundant), the handle answers "whose is this" for cleanup and bots (renovate/ already does this), and agent-std's issue number is worth keeping — it links branch → ticket without a lookup. Best of each; the one thing dropped is the redundant type. -- **"Squash to logical commits whose messages carry rationale and `Closes` trailers" (agent-std) vs squash-merge-to-one-commit (Mondeto, ruleset).** The repo squashes with PR title as subject and PR body as message, so the *PR body* is where rationale, verification, limits, and trailers live — the same content agent-std wants, in the place the platform actually preserves. Intermediate commits on the branch can be whatever helps you; they don't reach `main`. -- **PR template shape** — agent-std proposes What & why / Scope-not / Judgement calls / Verification / Merge order. Ours already had the first two, verification, and stacking; the missing piece was **Judgement calls**, now its own section. Nothing dropped. -- **"One issue per problem" vs "cluster findings"** — composer §1 vs its own §7 retrospective, and hardening's "one priority per issue". Not actually opposed: cluster by *fix boundary* (same-diff test) and split by *schedule/priority* (different-schedule test). Both tests are in §1. -- **Required review-thread resolution (kit v2 ruleset) vs "approve-with-nits means mergeable now" (hardening).** Both superseded. The ruleset rule stays dropped — mechanical thread resolution is click-blocking, not review quality. But the approve-with-nits verdict is gone too: no approval while any finding is open, however small. Every review point ends as a fix commit, a *filed* follow-up issue, or an explicit won't-do agreed in the thread (§4, §5). -- **"Require branch up to date" (mondeto lockfile incidents) vs the parallelism tax of many small PRs on shared files (composer §7).** Kept strict up-to-date — the lockfile incidents broke production twice, the tax is a click and a CI run. If the queue gets deep (>~5 concurrent PRs on shared files), enable GitHub **merge queue**, which performs the up-to-date test automatically and removes the clicks. Auto-merge on. +Places where reasonable conventions pulled in different directions; the decision and the reasoning: + +- **Branch naming.** Candidates: `feat/`, `/`, `type/-`. Chose `/-`: the type already lives in the Conventional-Commit PR title (so a type prefix is redundant), the handle answers "whose is this" for cleanup and bots (`renovate/` already works this way), and the issue number links branch → ticket without a lookup. +- **"Squash to logical commits whose messages carry rationale and trailers" vs squash-merge-to-one-commit.** The repo squashes with PR title as subject and PR body as message, so the *PR body* is where rationale, verification, limits, and trailers live — the same content, in the place the platform actually preserves. Intermediate commits on the branch can be whatever helps you; they don't reach `main`. +- **PR template shape.** What & why / Scope-not / Judgement calls / Verification / Merge order. The one section teams usually lack is **Judgement calls** — it's now its own section. +- **"One issue per problem" vs "cluster findings".** Not actually opposed: cluster by *fix boundary* (same-diff test) and split by *schedule/priority* (different-schedule test). Both tests are in §1. +- **Required review-thread resolution (ruleset) vs approve-with-nits.** Both rejected. Mechanical thread resolution is click-blocking, not review quality — the ruleset doesn't require it. But approve-with-nits is gone too: no approval while any finding is open, however small. Every review point ends as a fix commit, a *filed* follow-up issue, or an explicit won't-do agreed in the thread (§4, §5). +- **"Require branch up to date" vs the parallelism tax of many small PRs on shared files.** Kept strict up-to-date — stale-head merges have broken production; the tax is a click and a CI run. If the queue gets deep (>~5 concurrent PRs on shared files), enable GitHub **merge queue**, which performs the up-to-date test automatically and removes the clicks. Auto-merge on. - **Squash-only vs stacked PRs.** Squash makes stacks slightly more work (second-lander rebases `--onto main`). Kept squash — the clean linear log and title-as-commit are worth more than stack convenience; keep stacks ≤ 2 deep and prefer independent branches off `main`. -- **Priority labels** — kit v1 `P0/P1/P2` vs Mondeto `priority:*`. Chose `priority:critical|high|medium|low` (already in use, and the word "critical" carries its own definition; P0 doesn't). -- **Force-push** — Mondeto "never respond with a silent force-push" vs squash-merge culture where rebasing your own branch is normal. Resolution: force-pushing *your own PR branch* is fine; the rule is *announce what changed*. Never force-push shared branches or `main` (ruleset blocks it anyway). -- **Review effort** — composer's "both passes for every substantive review" vs its own retrospective "uniform max-rigour was the real cost driver". Chose tiered (§4); a real CI suite is what makes the light tier safe, which is why CI came first. +- **Priority labels.** `priority:critical|high|medium|low` over `P0/P1/P2` — the word "critical" carries its own definition; P0 doesn't. +- **Force-push.** "Never respond with a silent force-push" vs squash culture where rebasing your own branch is normal. Resolution: force-pushing *your own PR branch* is fine; the rule is *announce what changed*. Never force-push shared branches or `main` (ruleset blocks it anyway). +- **Review effort.** Uniform max-rigour review vs cost. Chose tiered (§4); a real CI suite is what makes the light tier safe, which is why CI comes first. diff --git a/.claude/shared/money-path-checklist.md b/.claude/shared/money-path-checklist.md index 3770bffb4..df39c0918 100644 --- a/.claude/shared/money-path-checklist.md +++ b/.claude/shared/money-path-checklist.md @@ -1,22 +1,22 @@ # Recurring-defect checklist — run against any security- or money-path diff -Each item caught at least one real finding (hardening PRs #49–#82, mondeto, agent-assisted-dev rounds). Reviewers and `/review-pr` run this item by item; authors run it before opening the PR. +Every item here caught at least one real finding in our own products. Reviewers and `/review-pr` run this item by item; authors run it before opening the PR. -1. **Self-referential verification** — is X checked against a value derived from X? (#75: receipt's token address compared to the log's own address.) Anchor checks to config/allowlists, never to the untrusted input. -2. **Defaults on the money path** — any `?? fallback` where the fallback has value? (#75: unknown token `?? "USDC"`.) Unknown means reject-and-alert, never a default. -3. **Shared keyspaces** — do two limiters/caches/locks build the same key? (#82: `ip:` shared by two throttles with different tiers.) Namespace per purpose. -4. **One-shot checks** — does a safety check latch on first failure and never retry? (#75: decimals check latched false on a transient RPC error.) Distinguish "refuted" from "unreachable"; only refuted may latch. -5. **Unbounded wedge + alert flood** — can one poisoned item pin a cursor or queue forever, and does its alert re-fire every tick? Bound retries, dedup alerts. (#75/#78; two wedge incidents in that codebase.) -6. **Silent-stall siblings** — for every loud failure path, is there a quiet branch that resets the failure counter? (#78: pending-log branch stalled with no alert while the tick "succeeded".) -7. **Upgrade path on existing state** — migrations against a populated DB, deploys against a root-owned volume, compose changes on a box mid-layout-change. Fresh-state testing proves nothing about the box you have. (#74, #77.) -8. **Cap semantics** — does the limit bind the actual resource or a client claim? (#82: `Content-Length` checked, chunked bodies still fully buffered.) Name which layer holds the real bound. -9. **Docs/code drift** — do README, bootstrap, examples, and error strings still describe the world this diff creates? (#74 README; #76's stale "recipient is not the treasury".) -10. **Test-double honesty** — does every fake honour the arguments it receives, and can every "never happened" assertion actually fail? (#76/#78 filter-ignoring fake; #82 vacuous counter.) -11. **Payout parity** — anything that ranks or pays: compared byte-for-byte against the other side (contract `fee = price * bps / 10000`, live `feeRate()`, the other route), including the *window* handed to an identical comparator (mondeto #48, #223, #224). -12. **Fee omission in every path** — if a fee/discount is subtracted in one code path, is it subtracted in all paths that serve users? (mondeto #182: log-scan path fixed, subgraph path — production — identical defect.) -13. **Error-body leakage** — what does each 4xx/5xx reveal? Exact scores are gradient oracles; flags are detection oracles. Fail-open or fail-closed decided per check, written down, and matched by the code (agent-std). -14. **Read bounds on write paths** — every read is bounded at the query (`take(limit)`), never collect-then-slice; no fail-open catch around an unbounded read (agent-std). -15. **Silent config fallbacks** — no `?? default` for critical config; unset/unrecognised is an error. Env compared exactly (`printf`, not `echo`); build-inlined vars need a rebuild; secrets checked for presence, never printed (agent-std). -16. **Guarantee-on-every-path** — for each prose guarantee ("never reveals X", "payment flow untouched"), point at the test asserting it on the *rejected/failure* path (agent-std). -17. **Threshold bands and boundaries** — the band between two thresholds, `>=` vs `>` at exact values, Unicode/empty input, concurrent duplicate requests (agent-std). +1. **Self-referential verification** — is X checked against a value derived from X? (Seen: a receipt's token address compared to the log's own address.) Anchor checks to config/allowlists, never to the untrusted input. +2. **Defaults on the money path** — any `?? fallback` where the fallback has value? (Seen: unknown token `?? "USDC"`.) Unknown means reject-and-alert, never a default. +3. **Shared keyspaces** — do two limiters/caches/locks build the same key? (Seen: `ip:` shared by two throttles with different tiers, so alternating requests degraded the stricter limit to the looser one.) Namespace per purpose. +4. **One-shot checks** — does a safety check latch on first failure and never retry? (Seen: a decimals check latched false on a transient RPC error.) Distinguish "refuted" from "unreachable"; only refuted may latch. +5. **Unbounded wedge + alert flood** — can one poisoned item pin a cursor or queue forever, and does its alert re-fire every tick? Bound retries, dedup alerts. +6. **Silent-stall siblings** — for every loud failure path, is there a quiet branch that resets the failure counter? (Seen: a pending-log branch stalled with no alert while the tick "succeeded".) +7. **Upgrade path on existing state** — migrations against a populated DB, deploys against a root-owned volume, compose changes on a box mid-layout-change. Fresh-state testing proves nothing about the box you have. +8. **Cap semantics** — does the limit bind the actual resource or a client claim? (Seen: `Content-Length` checked while chunked bodies were still fully buffered.) Name which layer holds the real bound. +9. **Docs/code drift** — do README, bootstrap, examples, and error strings still describe the world this diff creates? Stale docs asserting a property that doesn't hold are worse than silence — they stop anyone checking. +10. **Test-double honesty** — does every fake honour the arguments it receives, and can every "never happened" assertion actually fail? (Seen: a filter-ignoring fake; a counter assertion on a path where the counter can't increment.) +11. **Payout parity** — anything that ranks or pays: compared byte-for-byte against the other side (contract `fee = price * bps / 10000`, the live on-chain rate, the other route), including the *window* handed to an identical comparator — two identical comparators can still disagree when their block windows differ. +12. **Fee omission in every path** — if a fee/discount is subtracted in one code path, is it subtracted in all paths that serve users? (Seen: the fallback path fixed while the production path kept the identical defect.) +13. **Error-body leakage** — what does each 4xx/5xx reveal? Exact scores are gradient oracles; flags are detection oracles. Fail-open or fail-closed decided per check, written down, and matched by the code. +14. **Read bounds on write paths** — every read is bounded at the query (`take(limit)`), never collect-then-slice; no fail-open catch around an unbounded read. +15. **Silent config fallbacks** — no `?? default` for critical config; unset/unrecognised is an error. Env compared exactly (`printf`, not `echo`); build-inlined vars need a rebuild; secrets checked for presence, never printed. +16. **Guarantee-on-every-path** — for each prose guarantee ("never reveals X", "payment flow untouched"), point at the test asserting it on the *rejected/failure* path. +17. **Threshold bands and boundaries** — the band between two thresholds, `>=` vs `>` at exact values, Unicode/empty input, concurrent duplicate requests. 18. **Has it run in anger?** — for a path that moves real money / sends real messages / writes to a third party: was it exercised in production via **tester mode** (restricted audience, identical pipeline, small stake, verified at the source of truth) before public launch? If the app has no tester mode yet, that's the first ticket — see `tester-mode-pattern.md`. And `grep` the audience flag: it must not appear downstream of access control. diff --git a/.claude/shared/tester-mode-pattern.md b/.claude/shared/tester-mode-pattern.md index 3b2232fa4..31b2a9364 100644 --- a/.claude/shared/tester-mode-pattern.md +++ b/.claude/shared/tester-mode-pattern.md @@ -6,7 +6,7 @@ Synced from `pm-kit`. Generic pattern; the worked example is mini-quiz (see end) Some paths cannot be meaningfully faked: anything that moves money, spends gas, sends a real message, or writes to a third-party system. A staging environment proves the code typechecks against a mock; it does not prove that real gas gets spent from the real treasury and the real receipt comes back. Tester mode lets you run the *identical* production pipeline with real (small) stakes, visible only to a handful of internal accounts. -It has already paid for itself: the first native-CELO payout in mini-quiz was run this way (#15) and surfaced three bugs (#19, #20, #21) that no offline test could have found — all three lived in the gap between "the code typechecks" and "real gas was spent from the real treasury". +It has already paid for itself: our first native-CELO payout ran this way and surfaced three bugs that no offline test could have found — all three lived in the gap between "the code typechecks" and "real gas was spent from the real treasury". ## The model: restrict the audience, never the behaviour diff --git a/.github/ISSUE_TEMPLATE/user_story.yml b/.github/ISSUE_TEMPLATE/user_story.yml index e29630298..ba72f90ab 100644 --- a/.github/ISSUE_TEMPLATE/user_story.yml +++ b/.github/ISSUE_TEMPLATE/user_story.yml @@ -33,7 +33,7 @@ body: id: out-of-scope attributes: label: Non-goals - description: What this deliberately does NOT include, and who owns the adjacent work. ("User-facing copy belongs to #193.") Also say if this is explicitly NOT a blocker. + description: What this deliberately does NOT include, and who owns the adjacent work. ("User-facing copy belongs to #12.") Also say if this is explicitly NOT a blocker. validations: required: true - type: textarea @@ -41,7 +41,7 @@ body: attributes: label: Depends on / blocks description: Ordering that must survive without anyone remembering it. - placeholder: "Depends on #215. Blocks #216." + placeholder: "Depends on #12. Blocks #34." validations: required: false - type: dropdown From b354a61653d7e57e67b20d63436164c0b2b8a43c Mon Sep 17 00:00:00 2001 From: GigaHierz Date: Mon, 17 Aug 2026 23:38:04 +0100 Subject: [PATCH 3/3] fix: make CLAUDE.md MDX-parseable for the broken-links check Mintlify parses every .md in the repo as MDX; the HTML comment from the template is invalid MDX and failed the check. Blockquote instead. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6d6409e10..0668f5457 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,7 +1,7 @@ # CLAUDE.md — docs - +> Repo-owned. Keep it lean: a router. Shared rules are imported below and synced from pm-kit — +> don't restate them here. Every Claude session (local, Cowork, CI action) reads this. ## What this project is