diff --git a/.claude/skills/merge-pr/SKILL.md b/.claude/skills/merge-pr/SKILL.md new file mode 100644 index 00000000..a867855d --- /dev/null +++ b/.claude/skills/merge-pr/SKILL.md @@ -0,0 +1,58 @@ +--- +name: merge-pr +description: Use when merging or updating a pull request on GeniusWallet — rebase is the default strategy, with the cases where it is not. +--- + +# Merging + +## Rebase first + +Rebase is this project's default merge strategy. Keep history linear: a +branch that is behind `develop` gets rebased onto it, not merged from it. + +``` +git fetch origin +git rebase origin/develop +git push --force-with-lease # never --force +``` + +`--force-with-lease` refuses to overwrite work that arrived after your last +fetch. Plain `--force` does not, which is how a teammate's commit disappears. + +Merging a PR: + +``` +gh pr merge --rebase --delete-branch +``` + +**This is a change from what the history shows.** PRs #217 through #221 all +landed as merge commits (`Merge pull request #NNN from ...`). All three merge +methods are still enabled on the repo, so the setting will not stop anyone — +the convention is what does. + +## When rebase is the wrong tool + +- **A shared branch.** If anyone else has the branch checked out, rebasing + rewrites commits under them. Coordinate, or merge instead. +- **A long branch with conflicts in most commits.** Replaying forty commits + through the same conflict is worse than resolving it once. Merge, and say + why in the PR. +- **Branches with no shared history.** Rebase cannot help. Cherry-pick onto a + fresh branch off the target. + +State which one applies rather than switching silently. + +## Never + +- Force-push a branch you do not own without asking its owner. +- Rebase or push anything on `main` or `develop` directly. They take merges + from PRs only. +- Skip hooks (`--no-verify`) or bypass signing. If a hook fails, fix the cause. +- Merge with a red CI or a failing gate, however unrelated it looks. An + unrelated failure is still a failure somebody has to explain later. + +## Before merging + +Re-run the checks on the rebased tip, not on the pre-rebase commits — a clean +rebase can still produce a broken tree when two branches touched the same +behaviour. The list is in the `open-pr` skill. diff --git a/.claude/skills/open-pr/SKILL.md b/.claude/skills/open-pr/SKILL.md new file mode 100644 index 00000000..22dc1edd --- /dev/null +++ b/.claude/skills/open-pr/SKILL.md @@ -0,0 +1,74 @@ +--- +name: open-pr +description: Use when opening a pull request on GeniusWallet — covers branching, commit shape, the verification that must pass first, and how the PR description should read. +--- + +# Opening a PR + +## Branch + +Branch off `develop`, never `main`. `main` is the release branch; `develop` is +where work integrates. + +``` +git checkout develop && git pull && git checkout -b / +``` + +Check `git config user.email` resolves to the account that owns the work. +GitHub attributes commits by email, not by name — an inherited identity +credits the wrong person, and a protected branch can make that unfixable. + +## Before you open it + +All of these, actually run, output quoted — not assumed: + +``` +dart format lib test +flutter analyze # exits non-zero on infos, by design +flutter test +bash tool/check_brace_style.sh +bash tool/check_raw_colors.sh +bash tool/check_onboarding_seed_safety.sh +bash tool/check_no_new_key_logging.sh --scan-tree +bash tool/check_agent_rules_sync.sh +``` + +The Flutter SDK is not on `PATH` by default in this repo's environment. + +Never claim a baseline you did not run. If something fails, say so with the +output. + +## Commits + +One commit per concern, each standing on its own. The message says what +changed for a user and why, not which files moved — `git diff` already knows +the files. + +No tool attribution: no `Co-Authored-By` trailers for AI assistants, no +"generated with" footers, no bot emoji. This applies to commit messages, PR +descriptions, PR comments and release notes. + +## The description + +Written for a reviewer deciding whether to trust the change, not for a machine +summarising it. + +- **Lead with what a user hits.** "The order list could never show anyone + their orders" beats "refactored OrdersCubit". +- **One short verification section.** Test count, analyzer state, gates. +- **A "deliberately not here" list.** What you found and chose not to fix, with + the reason. This is what stops a reviewer hunting for something you already + considered. +- No walls of implementation detail. If a decision needs three paragraphs, it + belongs in a code comment or the commit message. + +## Open it as a draft + +``` +gh pr create --draft --base develop --title "" --body-file <file> +``` + +Draft by default. Mark it ready when CI is green and you have re-read the +diff yourself. Opening non-draft is the exception, not the norm. + +Do not open a PR without the author's explicit go-ahead. diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md new file mode 100644 index 00000000..1e65c280 --- /dev/null +++ b/.claude/skills/review-pr/SKILL.md @@ -0,0 +1,81 @@ +--- +name: review-pr +description: Use when reviewing a pull request on GeniusWallet — the project rules a reviewer checks against, in the order they cause real damage. +--- + +# Reviewing a PR + +Check the diff against these, hardest consequence first. `AGENTS.md` is the +full rule set; this is what a reviewer actually looks for. + +## Wallet safety — none of these are style preferences + +- A private key or mnemonic **must not** be a field on a Cubit/Bloc state + class. States are equatable, printable, and reach `BlocObserver` logs. +- Nothing derived from a seed phrase or key gets logged, `toString()`d or sent + to Sentry. +- `Random.secure()` only. A plain `Random()` in key generation is how a real + Flutter wallet shipped a 32-bit key. +- Prefer `Uint8List` over `String` for secrets — a `String` is immutable and + cannot be zeroed. +- Anything holding key material must be disposed, and disposed by an owner + that can (a `StatelessWidget` cannot). + +## Correctness the tests will not catch + +- **Does it work on mobile?** `android/` and `ios/` are real targets. Platform + APIs behave differently — e.g. from Android API 29 an app without focus can + neither read nor write the clipboard, so a background timer touching it does + nothing at all. +- **Does a test pin the defect instead of the fix?** A passing assertion can + be holding a bug in place. Ask what the test would do if the bug were fixed. +- **Does a test anchor on an accident?** Locating a widget by error text that + only appears because a fetch failed will break the moment the fetch works. + +## Accessibility — explicitly not something this repo is lazy about + +- Every interactive control reachable and operable from a keyboard + (WCAG 2.1.1, Level A). A bare `GestureDetector` is not. +- Colour contrast meets AA in **both** appearance modes and in every state. + Light mode is where this repo has historically broken. +- Disabled states stay visibly distinct. + +## Design system + +- Colours and spacing from tokens, via `Theme.of(context).extension<GWColors>()`. + No `Colors.*` or `Color(0x…)` outside `lib/theme/` — `Colors.transparent` is + the one permitted exception. +- Never cache a theme-derived value in a long-lived object; re-read it in + `build`. +- Extract a `StatelessWidget`, never a `_buildFoo()` returning a `Widget`. +- Widgets do not reach past the repository layer: no `Hive.box`, `File`, + `http` or direct SDK calls in a widget or its `State`. Go through a + bloc/cubit. + +## Shape of the change + +- **Was it needed at all?** YAGNI. Then: does the stdlib do it? A native + platform feature? An already-installed dependency? Only then write it. + Reimplementing something Material already provides is the common miss. +- **Rule of Three.** Two occurrences do not justify a shared component; three + do. If the shared version needs a boolean flag to serve both callers, or you + cannot name it clearly, it should not exist. +- **Braces on every `if`**, body on its own line. `tool/check_brace_style.sh` + enforces it. +- **Intentional shortcuts carry a `ponytail:` comment** naming the ceiling and + the upgrade path. +- **Comment length in proportion to the code.** A 7-line function does not + need 27 lines of preamble; that history belongs in the commit message. + +## Verification + +The PR must state a baseline someone actually ran — `dart format`, +`flutter analyze`, `flutter test`, and the `tool/*.sh` gates. Treat an +unquoted claim as unverified. + +## How to give the feedback + +Say what is wrong and why it matters, in a sentence or two. A short question +often lands better than an assertion — "Does this work on mobile?" found a +real bug in PR #222 in six words. Reserve blocking for things that break +users, security or accessibility; everything else is a comment. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..d81d61a9 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,136 @@ +<!-- + GENERATED FILE - DO NOT EDIT. + + Source: AGENTS.md at the repo root. Edit that, then run: + bash tool/check_agent_rules_sync.sh --fix + + This copy exists because GitHub Copilot's editor integrations (VS Code and + Copilot for Xcode) read .github/copilot-instructions.md automatically, while + their AGENTS.md support is experimental and off by default. Claude Code and + opencode read AGENTS.md directly and need no copy. + + See docs/ai-agents.md for the full picture. +--> + +> **This file is the single source of truth for every AI agent on this repo.** +> Claude Code, opencode and Copilot's coding agent read it directly. +> `.github/copilot-instructions.md` is a GENERATED copy for Copilot in VS Code +> and Xcode — edit this file, then run `bash tool/check_agent_rules_sync.sh --fix`. +> Workflow skills (opening a PR, merging, reviewing) live in `.claude/skills/`, +> which Claude Code and opencode both read. See `docs/ai-agents.md`. + +You are a lazy senior developer. Lazy means efficient, not careless. The best code is the code never written. + +Before writing any code, stop at the first rung that holds: + +1. Does this need to be built at all? (YAGNI) +2. Does the standard library already do this? Use it. +3. Does a native platform feature cover it? Use it. +4. Does an already-installed dependency solve it? Use it. +5. Only then: write the minimum code that works. + +Rules: + +- No abstractions that weren't explicitly requested. +- No new dependency if it can be avoided. +- No boilerplate nobody asked for. +- Deletion over addition. Boring over clever. Fewest files possible. +- Question complex requests: "Do you actually need X, or does Y cover it?" +- Pick the edge-case-correct option when two stdlib approaches are the same size, lazy means less code, not the flimsier algorithm. +- Mark intentional simplifications with a `ponytail:` comment. If the shortcut has a known ceiling (global lock, O(n²) scan, naive heuristic), the comment names the ceiling and the upgrade path. + +Not lazy about: input validation at trust boundaries, error handling that prevents data loss, security, accessibility, the calibration real hardware needs (the platform is never the spec ideal, a clock drifts, a sensor reads off), anything explicitly requested. Lazy code without its check is unfinished: non-trivial logic leaves ONE runnable check behind, the smallest thing that fails if the logic breaks (an assert-based demo/self-check or one small test file; no frameworks, no fixtures). Trivial one-liners need no test. + +Do not create commits. +Files under `/banxa` and `/squidrouter` are auto-generated. Do not change them. + +## Dart coding standards + +Baseline is Effective Dart + `flutter_lints`; `dart format` owns all whitespace. Only the rules +below are non-obvious, project-specific, or stricter than the tooling — everything else you can +infer from the code. Enforcement lives in `analysis_options.yaml`, `tool/*.sh` and CI, not here. + +**YOU MUST brace every `if`, with the body on its own line.** + +```dart +if (!mounted) { return; } // NO — one line +if (!mounted) // NO — no braces + return; + +if (!mounted) { // YES + return; +} +``` + +Same-line `{` is correct — Allman style is *not* wanted. Two reasons this is a hard rule: you cannot +set a breakpoint on the true-branch otherwise, and a `log()` almost always ends up in there later. +No Dart lint can express this (`curly_braces_in_flow_control_structures` is already on and permits +the one-line form); `tool/check_brace_style.sh` is the enforcement. + +**Widgets, not helper methods.** Extract to a `StatelessWidget`, never a `_buildFoo()` returning a +`Widget`. A helper rebuilds the whole enclosing widget, can't be `const`, and is invisible to the +DevTools inspector. + +**Rule of Three for extraction.** Two occurrences do not justify a shared component; three do. +Duplication is cheaper than the wrong abstraction. If the shared version needs a boolean flag to +serve both callers, or you can't name it clearly, don't extract it. + +**Colours and spacing come from tokens.** Read via `Theme.of(context).extension<GWColors>()`. +No `Colors.*` or `Color(0x…)` outside `lib/theme/`. Every colour must be correct in **both** +appearance modes and meet WCAG AA — light mode is where this repo has historically broken. +Never cache a theme-derived value in a long-lived object; re-read it inside `build`. + +**Widgets do not reach past the repository layer.** No `Hive.box(…)`, `File`/`Directory`, `http`, +or direct SDK calls inside a widget or its `State`. Go through a bloc/cubit → repository. Flutter's +own guidance: *"Views shouldn't contain any business logic."* + +**Wallet safety — these are not style preferences:** +- A private key or mnemonic MUST NOT become a field on a Cubit/Bloc state class. States are + equatable, printable, and land in `BlocObserver` logs by default. +- Never log, `toString()`, or send to Sentry anything derived from a seed phrase or key. +- `Random.secure()` only. A plain `Random()` in key generation is how a real Flutter wallet + (Proton) shipped a 32-bit key. +- Prefer `Uint8List` over `String` for secrets — `String` is immutable and cannot be zeroed. + +**Before you call anything done:** `dart format`, `flutter analyze` (it exits non-zero on infos — +that is intentional), and `flutter test`. Quote real output; never claim a baseline you didn't run. +Note the Flutter SDK is not on `PATH` by default in this repo's environment. + +## Working in parallel sessions + +Two or more Claude sessions may run against this repo at once. On 2026-07-22 two sessions collided +in five measured ways: sketch numbers clashed twice (016, 020), `ROADMAP.md`/`STATE.md`/`MANIFEST.md` +could not be split when committing, `HANDOFF.json` held one slot for two sessions, the test baseline +drifted 187→222 so every agent misread a neighbour's tests as a regression, and two `flutter run` +instances fought over the Hive container lock. + +The pattern behind all five: **a file is the unit of conflict.** One file per item is safe. One +shared file is not. + +**Roles.** Exactly one session is the EXECUTOR. Everything else is a DESIGN or RESEARCH session. + +**Only the executor may:** +- commit, stage, push, or touch git state in any way +- run `flutter run` (a second instance dies on the Hive lock at + `~/Library/Containers/ai.gnus.GeniusWallet.jakub/`) +- run the full `flutter test` suite and quote a baseline +- write `.planning/ROADMAP.md`, `.planning/STATE.md`, `.planning/sketches/MANIFEST.md`, + `.planning/HANDOFF.json` +- edit anything under `lib/`, `test/`, `macos/`, `packages/` + +**A design session may only** create `.planning/sketches/<its own range>/` and append single files to +`.planning/todos/pending/`. That is the queue: one file per item, never a shared list. + +**Sketch number ranges are reserved, not first-come.** Execution 000-099 · design lane A 100-149 · +design lane B 150-199. A shared counter has now collided on two consecutive days. + +**Every session writes its own `.planning/HANDOFF-<topic>.md` before it ends.** On 2026-07-22 this +was the only reason one session's work could be summarised by another. A session that ends without +one has produced no day summary. + +**A parallel agent's claim that "a concurrent session changed the tree" is a hypothesis, not a fact.** +Every such report on 2026-07-22 turned out to be a sibling from the same wave or a stale git snapshot +in the agent's own prompt. Check `git reflog` before acting on one. + +**If a design session must touch code or run the app, give it its own worktree** — +`git worktree add ../GW-<lane> <branch>` — not a second checkout of the same tree. diff --git a/AGENTS.md b/AGENTS.md index 0d3d8dfb..fedc0dd6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,3 +1,10 @@ +> **This file is the single source of truth for every AI agent on this repo.** +> Claude Code, opencode and Copilot's coding agent read it directly. +> `.github/copilot-instructions.md` is a GENERATED copy for Copilot in VS Code +> and Xcode — edit this file, then run `bash tool/check_agent_rules_sync.sh --fix`. +> Workflow skills (opening a PR, merging, reviewing) live in `.claude/skills/`, +> which Claude Code and opencode both read. See `docs/ai-agents.md`. + You are a lazy senior developer. Lazy means efficient, not careless. The best code is the code never written. Before writing any code, stop at the first rung that holds: diff --git a/docs/ai-agents.md b/docs/ai-agents.md new file mode 100644 index 00000000..72a56202 --- /dev/null +++ b/docs/ai-agents.md @@ -0,0 +1,98 @@ +# AI coding agents in this repo + +Different people here drive this repo with different agents, and they do not +all read the same file. This page says what your agent needs in order to pick +up the project rules, and where to put a new rule. + +## One source of truth + +**`AGENTS.md` at the repo root.** It is the only rules file anyone edits by +hand. + +| Agent | What it reads | What you have to do | +|---|---|---| +| Claude Code | `CLAUDE.md`, one line: `@AGENTS.md` | Nothing. Works on clone. | +| opencode | `AGENTS.md` directly, in preference to `CLAUDE.md` | Nothing. Works on clone. | +| Codex | `AGENTS.md` directly | Nothing. Works on clone. | +| Copilot coding agent, Copilot code review | `AGENTS.md` directly | Nothing. Works on clone. | +| Copilot in VS Code | `.github/copilot-instructions.md`, auto-detected | Nothing. Optionally enable the experimental `AGENTS.md` setting to read the source instead. | +| Copilot for Xcode | `.github/copilot-instructions.md` | Nothing. Same generated file. | +| Cursor, Cline, Gemini CLI, Continue, others | Their own directory formats | Not generated today — see "Adding an agent" below. | + +Most agents read `AGENTS.md` already. Copilot's **editor** integrations are the +exception: `AGENTS.md` support in VS Code is experimental and off by default, +while `.github/copilot-instructions.md` is picked up automatically. + +So `.github/copilot-instructions.md` is a **generated copy** of `AGENTS.md`. +It carries a do-not-edit header. + +### Adding or changing a rule + +``` +# 1. edit AGENTS.md +# 2. regenerate the Copilot copy +bash tool/check_agent_rules_sync.sh --fix +# 3. commit both +``` + +`tool/check_agent_rules_sync.sh` (no argument) fails if the two have drifted. +Run it with the other gates before opening a PR. + +## Adding an agent + +If you start using a tool that reads neither `AGENTS.md` nor +`.github/copilot-instructions.md`: + +1. Find the file or directory it expects (e.g. `.cursor/rules/`, + `.clinerules/`, `.github/instructions/*.instructions.md`). +2. Add it as a second target in `tool/check_agent_rules_sync.sh` — the render + function emits a header plus `AGENTS.md` verbatim, so a new target is a + path and a header, not new logic. +3. Run `--fix`, commit the generated file, and note it in the table above. + +If the count of generated targets gets past two or three, stop and reach for a +purpose-built tool instead (Ruler, rulesync, AgentSync). One file does not +justify a dependency; four might. + +## Skills + +`.claude/skills/<name>/SKILL.md`. **Claude Code and opencode both read that +path natively** — opencode loads `.claude/skills/*/SKILL.md` alongside its own +`.opencode/skills/` and `.agents/skills/`. Copilot's agent mode reads +`SKILL.md` too. One directory, no sync, nothing to configure. + +If your agent supports skills but looks somewhere else, point it at +`.claude/skills/` rather than copying the files — a second copy is a second +thing to keep in step. + +Currently: + +| Skill | For | +|---|---| +| `open-pr` | Branching, commit shape, the checks that must pass, how the description should read, opening as a draft | +| `merge-pr` | Rebase-first merging and the cases where it is the wrong tool | +| `review-pr` | What a reviewer checks, hardest consequence first | + +A skill here must be **self-contained** — no references to files outside the +repo. A skill that reads `$HOME/...` works on one machine and silently does +nothing on everyone else's. + +## What is deliberately not here + +- **No rules-sync package.** Ruler, rulesync and AgentSync all solve this, and + all are the wrong size for one generated file in a repo with no JS + toolchain. Revisit if the team adds Cursor, Cline or Gemini, each of which + brings its own directory format. +- **No vendored GSD skills.** 59 of the 69 load workflows from + `$HOME/.claude/gsd-core/`, so copying them here produces skills that break + on every machine but the one they came from. Anyone who wants that workflow + installs GSD themselves. +- **Nothing for Xcode's built-in Coding Intelligence.** It has no repo-level + rules convention, so there is no file to generate for it. Copilot for Xcode + reads the generated file and is covered. + +## Models + +The models in use — DeepSeek, GLM, Xiaomi MiMo, Claude — do not affect any of +this. Rules files are read by the **harness**, not the model. Pointing +opencode at a different provider changes nothing here. diff --git a/tool/check_agent_rules_sync.sh b/tool/check_agent_rules_sync.sh new file mode 100644 index 00000000..792f3026 --- /dev/null +++ b/tool/check_agent_rules_sync.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# +# tool/check_agent_rules_sync.sh +# +# WHAT THIS IS FOR +# ---------------- +# The team drives this repo with four different AI coding agents, and they do +# not all read the same file: +# +# Claude Code CLAUDE.md, which is one line: `@AGENTS.md` +# opencode AGENTS.md natively (it prefers AGENTS.md over +# CLAUDE.md when both exist) +# Copilot coding agent AGENTS.md natively +# Copilot in VS Code .github/copilot-instructions.md +# Copilot for Xcode .github/copilot-instructions.md +# +# Three of those read AGENTS.md already. Only Copilot's editor integrations do +# not: AGENTS.md support in VS Code is experimental and off by default, while +# .github/copilot-instructions.md is picked up automatically. So exactly ONE +# file has to be kept in step with AGENTS.md, and this script is what keeps it +# honest. +# +# WHY A SCRIPT AND NOT A PACKAGE +# ------------------------------ +# Ruler, rulesync, AgentSync and friends all solve this, and all of them are +# the wrong size for it here: they add a Node dependency and a generate step to +# produce ONE derived file. This repo already has five shell gates in tool/ and +# no JS toolchain. If the team later adds Cursor, Cline or Gemini -- each with +# its own directory format -- revisit that decision; two or three targets is +# where a package starts paying for itself. +# +# WHY NOT A SYMLINK +# ----------------- +# A symlink would need no script at all, but it needs core.symlinks and, on +# Windows, Developer Mode. A checkout without those turns the link into a text +# file containing a path, which Copilot would read as the literal string +# "../AGENTS.md" and silently apply no rules at all. A copy plus a check fails +# loudly instead. +# +# USAGE +# ----- +# bash tool/check_agent_rules_sync.sh # verify; exit 1 on drift +# bash tool/check_agent_rules_sync.sh --fix # regenerate, then verify +# +# Run --fix after editing AGENTS.md. AGENTS.md is the ONLY file anyone edits by +# hand; .github/copilot-instructions.md is generated and carries a header +# saying so. + +set -uo pipefail +cd "$(git rev-parse --show-toplevel)" || exit 1 + +SOURCE="AGENTS.md" +GENERATED=".github/copilot-instructions.md" + +if [ ! -f "$SOURCE" ]; then + echo "FAIL: $SOURCE is missing -- it is the single source of truth for every agent." + exit 1 +fi + +# The generated file is the header plus AGENTS.md verbatim. Verbatim matters: +# a transform (section filtering, reformatting) would mean the rules an agent +# reads are not the rules a human reviewed. +render() { + cat <<'HEADER' +<!-- + GENERATED FILE - DO NOT EDIT. + + Source: AGENTS.md at the repo root. Edit that, then run: + bash tool/check_agent_rules_sync.sh --fix + + This copy exists because GitHub Copilot's editor integrations (VS Code and + Copilot for Xcode) read .github/copilot-instructions.md automatically, while + their AGENTS.md support is experimental and off by default. Claude Code and + opencode read AGENTS.md directly and need no copy. + + See docs/ai-agents.md for the full picture. +--> + +HEADER + cat "$SOURCE" +} + +if [ "${1:-}" = "--fix" ]; then + mkdir -p "$(dirname "$GENERATED")" + render > "$GENERATED" + echo "Regenerated $GENERATED from $SOURCE." +fi + +if [ ! -f "$GENERATED" ]; then + echo "FAIL: $GENERATED is missing. Run: bash tool/check_agent_rules_sync.sh --fix" + exit 1 +fi + +if ! diff -q <(render) "$GENERATED" >/dev/null 2>&1; then + echo "FAIL: $GENERATED has drifted from $SOURCE." + echo "" + echo " Copilot users (VS Code, Xcode) are reading different rules from" + echo " everyone else. Fix with:" + echo "" + echo " bash tool/check_agent_rules_sync.sh --fix" + echo "" + echo "Difference (expected vs actual):" + diff <(render) "$GENERATED" | head -40 | sed 's/^/ /' + exit 1 +fi + +echo "PASS: $GENERATED is in step with $SOURCE." +exit 0