From cc7a5ebc7e867c52571ef444090d124903ca89f8 Mon Sep 17 00:00:00 2001 From: "celo-template-sync[bot]" <318017900+celo-template-sync[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 11:53:41 +0000 Subject: [PATCH] chore(kit): synced file(s) with celo-org/pm-kit --- .claude/shared/engineering-rules.md | 10 +++++++--- .github/PULL_REQUEST_TEMPLATE.md | 5 ++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.claude/shared/engineering-rules.md b/.claude/shared/engineering-rules.md index a58d265cf..431e33b6d 100644 --- a/.claude/shared/engineering-rules.md +++ b/.claude/shared/engineering-rules.md @@ -95,9 +95,13 @@ The two rules that hold everything else together: **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`. -**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. +**Test in a normal browser, not only the primary wallet client — and on a phone viewport.** 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; every product is mobile-first, so the pass runs at a phone viewport as well as desktop. -**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. +**Two levels of UI verification, in this order.** Level one: the agent drives a headless browser over the changed surface — navigate, snapshot, interact, read console and network, screenshot, desktop and phone — fixes what it finds, and repeats until console errors are zero and the surface does what the issue says. `/write-pr` does this before a PR opens; `/review-pr` repeats it to refute. Level two: a person clicks through last, and should find nothing. A frontend PR carries the level-one evidence (routes, actions, console-error count, screenshots); a PR asking for review without it has skipped its own testing. What a browser pass cannot see — hover states, animations, anything between point A and point B — stays with level two. + +**Bigger changes on a payment path: one real transaction, requested as optional evidence.** A new flow, changed amounts, recipients, or fee logic gets asked for a tx hash — the author runs the flow once from their own wallet with a small amount, the reviewer verifies the receipt at the chain (status, from, to, value), and hash + status go in the PR body. It is optional, and a review that did not get one says so ("covered code and browser pass, not a live send") rather than implying it. No wallet keys are handed to agents or CI for this; the browser pass drives the UI up to the wallet prompt, a person signs. Real-money paths going public still run behind tester mode first (`tester-mode-pattern.md`). + +**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. `e2e-smoke` green means exactly: every listed route on the preview returned < 400 with visible text, no framework error overlay, zero console errors, zero uncaught exceptions, zero failed same-origin requests, on desktop and phone. It does not say the page is right, laid out right, or that any other path works. --- @@ -106,7 +110,7 @@ The two rules that hold everything else together: **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. +- *Logic, new surface, funds/security/release, wallet tree, anything rendered*: 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 with the browser tools (the preview or a dev server, desktop and phone viewport, console clean — never by reading JSX; for a CLI, the 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.** 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. diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index d39f0d23f..9df44f289 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -44,7 +44,9 @@ Refs #