Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 29 additions & 30 deletions .agents/commands/pr.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ If no base branch argument provided, detect the repo's default branch:
- Fetch 10 most recent PRs (open or closed) from the extracted repo for writing style reference
- Run `git log $base..HEAD --oneline` for commit messages
- Run `git diff $base...HEAD --stat` for understanding scope of changes
- List the journeys the branch adds or updates: `git diff --name-only --diff-filter=d $base...HEAD -- journeys | grep '\.xml$'`

### 4. Extract Linked Issues
Scan commits for issue references:
Expand Down Expand Up @@ -134,63 +135,61 @@ When the user provides custom instructions after `--`:
- Always use this structure:
```md
### QA Notes
#### Journeys
#### Manual Tests
#### Automated Checks
```
- Under `#### Journeys`, list every journey the branch adds or updates (Step 3), one per line as an unchecked checkbox (`- [ ] `), then `new` or `updated`, then the bare journey file name in backticks, then a dash and what the journey proves.
- Reference journeys by bare file name only, never the full path. Only when two listed journeys share the same name, prefix the shortest leading path segment(s) that disambiguate them, the same rule as test files.
- A PR with a user-visible change adds or updates the journey that proves it, and any journey whose route the diff changes; list them all. Reviewers drive the listed journeys on a device.
- `#### Journeys` takes one of two empty values: `N/A — no user-visible behaviour change.` when the diff changes nothing a user can see, and `N/A — not drivable; see Manual Tests.` when it does but every flow it touches needs a capability the Capabilities table in `journeys/README.md` does not list. The second value requires a matching step under `#### Manual Tests`.
- Leave every checkbox under `#### Journeys` and `#### Manual Tests` unchecked; the reviewer ticks a line after driving it on the PR head.
- When the diff changes user-visible behaviour and no journey covers it, stop and report the flows that need a journey. A flow the Capabilities table cannot provide is the exception: record it under `#### Manual Tests` naming the missing capability instead of stopping.
- Under `#### Manual Tests`, write a step only when it needs a capability the Capabilities table in `journeys/README.md` does not list, and name that capability in the step. Everything a journey can drive belongs under `#### Journeys`.
- Keep local verification commands, `xcodebuild`, Swift tests, SwiftFormat, translation validation, unit tests, build passes, cargo test, cargo clippy, npm test, typecheck, CI coverage, or similar automated checks out of `#### Manual Tests`; summarize them under `#### Automated Checks` when they add useful context.
- Use `#### Automated Checks` to summarize automated verification evidence, prioritizing coverage added, modified, or removed, each with the test file name and a short explanation.
- Use `#### Automated Checks` for a flat list of automated verification evidence, in the keyword order `added`, `updated`, `removed`, `ran`: the keyword, the bare test file name in backticks, then a dash and the behaviour it proves.
- Reference test files by bare file name only (e.g. `TransferViewModelTests.swift`), never the full path. Only when two referenced test files share the same name, prefix the shortest leading path segment(s) that disambiguate them (e.g. `BitkitTests/SendTests.swift` vs `BitkitUITests/SendTests.swift`).
- For removed automated coverage, state why it was removed.
- Do not list standard CI or PR bot commands as checkbox items just because they run for every PR. If standard CI coverage is worth mentioning, summarize it in one sentence.
- List raw commands only when they were run locally, are non-standard, use special flags or environment values, validate workflow behavior, or explain a meaningful verification gap.
- For a `removed` item, state why the coverage was removed.
- Use `ran` only for checks CI does not run. Do not list standard CI or PR bot commands just because they run for every PR.
- List raw commands under `ran` only when they were run locally, are non-standard, use special flags or environment values, validate workflow behavior, or explain a meaningful verification gap.
- For workflow behavior validation, include `(after merge)` in the automated check item because workflow changes only take effect for PRs opened after the workflow update merges.
- If no actionable manual validation exists, write `N/A` under `#### Manual Tests`.
- If no step needs a missing capability, write `N/A` under `#### Manual Tests`.
- If no automated checks were run and no automated coverage changed, write `N/A` under `#### Automated Checks`.
- Write manual tests using this template:
```md
- [ ] **{numbering}.** {optional_condition + →} {screen_action} → {next_screen_action}: expectation
- [ ] {optional_condition + →} {action} → {expectation} — {missing capability} not in Capabilities
```
- Use a list of unchecked checkboxes for each individual test.
- Use a numbered prefix for each test, in bold, for example `**1.**`, `**2.**`.
- Use `regression:` for regression checks, positioned after the numbering.
- Use sub-lists for variations of the same test.
- Use letter suffixes in numbering for each variation when a test has a sub-list, for example `**3a.**`, `**3b.**`.
- Always use `→` to denote navigation, for example `Send → Amount`.
- Use `regression:` for regression checks, positioned at the start of the action.
- Always use `→` to denote navigation, for example `Send → Amount`; the last `→` introduces the expectation.
- Use screen names from code, formatted as separate words without the `View` or `Screen` suffix, for example `FundReceiveView` becomes `Fund Receive`.
- Use short-form wording like `in-sheet` for sheet screens, `nav` for navigation, `back` for back nav, and `LN` for Lightning Network.

**For library repos (has `bindings/` directory or `Cargo.toml`):**
Structure manual QA around integration validation only. Automated checks belong under `#### Automated Checks`.
These repos have no `journeys/`, so the walkthrough stays under `#### Manual Tests` and needs no capability reason. Structure it around integration validation only. Automated checks belong under `#### Automated Checks`.

Example:
```
### QA Notes
#### Manual Tests
- [ ] **1.** Consumer app → exercise updated binding flow: behavior matches previous release.
- [ ] **2.** `regression:` iOS integration screen → trigger changed API path: no crash or stale data.
- [ ] Consumer app → exercise the updated binding flow → behavior matches the previous release
- [ ] `regression:` iOS integration screen → trigger the changed API path → no crash or stale data
#### Automated Checks
- Binding tests added: cover updated iOS API path in `bindings/ios/...`.
- CI: standard cargo and binding checks run by the PR bot.
- added `FooBindingTests.swift` — covers the updated iOS API path
- ran `cargo test --all-features` — binding round-trip CI does not build
```

Concrete style target:
```md
### QA Notes
#### Journeys
- [ ] new `send-amount-over-balance.xml` — error shows before the 15 s timeout
- [ ] updated `lightning-transfer-detail.xml` — Connection opens Channel Detail
#### Manual Tests
- [ ] **1.** No usable channels/spending balance → scan LN invoice: error shows immediately, not after 15s.
- [ ] **2.** Scanner → scan fixed amount LN invoice: Send Confirm or QuickPay opens directly.
- [ ] **3a.** `regression:` Send → scanner/paste fixed amount LN invoice: in-sheet nav to Confirm or QuickPay.
- [ ] **3b.** `regression:` Variable amount LN invoice/LNURL-pay: lands on Amount view.
- [ ] **4a.** Activity Detail of LN transfer → tap Connection: lands on Channel Detail.
- [ ] **4b.** back: returns to Activity Detail.
- [ ] **5a.** Settings → Lightning Connections → tap channel: still opens Channel Detail.
- [ ] **5b.** back: returns to Connections List.
- [ ] **6.** `regression:` Channel Detail → tap Close Connection: works.
- [ ] Pair a Trezor over BLE → Home shows the hardware wallet card — BLE pairing not in Capabilities
#### Automated Checks
- Unit tests added: cover invoice timeout handling in `TransferViewModelTests.swift`.
- Unit tests modified: update channel navigation assertions in `ChannelDetailsViewModelTests.swift`.
- Test coverage removed: delete stale mock-only assertions from `OldFlowTests.swift` because the flow no longer exists.
- CI: standard build and test checks run by the PR bot.
- added `TransferViewModelTests.swift` — rejects amounts over the spending balance
- updated `SendFlowTests.swift` — fixed-amount invoice skips the Amount screen
- removed `OldFlowTests.swift` — flow no longer exists
```

**Preview Section (conditional):**
Expand Down
8 changes: 6 additions & 2 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@

### QA Notes

#### Journeys
Comment thread
ovitrif marked this conversation as resolved.

<!-- One line per journey this PR adds or updates: `new` or `updated`, the bare journey file name in backticks, then what it proves — `- [ ] new `send-amount-over-balance.xml` — error shows before the 15 s timeout`; prefix the shortest disambiguating folder only when two journeys share a name. Two empty values: `N/A — no user-visible behaviour change.`, or `N/A — not drivable; see Manual Tests.` when every flow touched needs a capability the Capabilities table in `journeys/README.md` does not list. Leave the boxes unchecked; the reviewer ticks a line after driving it. -->

#### Manual Tests

<!-- Unchecked, numbered steps for the reviewer: action → expectation. `N/A` when there is nothing to verify by hand. -->
<!-- Only for a step needing a capability the Capabilities table in `journeys/README.md` does not list: action → expectation — the missing capability, as in `- [ ] Pair a Trezor over BLE → Home shows the hardware wallet card — BLE pairing not in Capabilities`. `N/A` when there is none. -->

#### Automated Checks

<!-- Coverage added, changed, or removed, by test file name; non-standard commands you ran. `N/A` when nothing changed. -->
<!-- Flat list in the keyword order `added`, `updated`, `removed`, `ran`: keyword, bare test file name, dash, the behaviour proven — `- added `TransferViewModelTests.swift` — rejects amounts over the spending balance`; `ran` only for what CI does not run. `N/A` when nothing changed. -->
25 changes: 17 additions & 8 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,19 @@ This app integrates with:
- **Electrum/Esplora** for blockchain data
- **Blocktank** for Lightning channel services

## Prerequisites

- The **`xcodebuildmcp` CLI and its simulator UI automation** are required, not recommended: they are
how an agent drives a running simulator, so without them the journeys under `journeys/` cannot run
and a PR's QA contract cannot be checked. Install the CLI as in the Agent CLI section below.

## Build & Development Commands

### Agent CLI (XcodeBuildMCP)

Agents should prefer the `xcodebuildmcp` CLI over raw `xcodebuild`, `xcrun`, and `simctl`. It wraps the
same toolchain, parses build output, and adds simulator UI automation (AXe is bundled — no separate install).
Agents drive the simulator with the `xcodebuildmcp` CLI rather than raw `xcodebuild`, `xcrun`, and
`simctl`. It wraps the same toolchain, parses build output, and adds simulator UI automation (AXe is
bundled — no separate install).

```bash
# Install
Expand Down Expand Up @@ -156,9 +163,9 @@ xcodebuildmcp simulator test

Separately from the test suites, `journeys/` holds XML walkthroughs of app behaviour that an agent
evaluates by driving a running simulator — number pad caps, notification permission, widget flows,
hardware wallet pairing and transfers. They are developer assistance rather than a test layer:
nothing runs them in CI and they gate nothing. Read `journeys/README.md` before running or writing
one, and see the Journeys section under Code Style & Conventions.
hardware wallet pairing and transfers. Nothing in `.github/workflows` runs them; reviewers drive the
ones a PR lists. Read `journeys/README.md` before running or writing one, and see the Journeys
section under Code Style & Conventions.

## Architecture

Expand Down Expand Up @@ -377,9 +384,11 @@ Ensure accessibility modifiers and labels are added to custom components.
`<description>` and the suite README — never assert Android behaviour iOS does not have.
- SKIP a journey only when the iOS feature does not exist, and record it under "Not ported" in
`journeys/README.md` with what is missing.
- Journeys are developer-assistance specs, not a QA gate. Nothing runs them in CI and no runner is
wired up for them; `ai-device-tests.yml` runs `TrezorBridgeDashboardUITests` and never reads
`journeys/`. An agent runs one on request.
- Journeys are **the QA contract for a PR**. A PR with a user-visible change adds or updates the
journeys that prove it and any journey whose route it changes, and lists them under `#### Journeys`
in the PR body. Reviewers drive the listed journeys on a device; nothing in `.github/workflows`
reads `journeys/`. Write a manual test only for a step that needs a capability the Capabilities
table in [`journeys/README.md`](journeys/README.md) does not list.
- A journey that disagrees with the app is most likely stale rather than evidence of a bug. Say what
you found and update the journey; escalate only once you have separately confirmed the app is wrong.

Expand Down
Loading
Loading