Skip to content
Open
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.
- `N/A — no user-visible behaviour change.` is the only empty value for `#### Journeys`.
- 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.
- 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 flowbehavior matches the previous release
- [ ] `regression:` iOS integration screen → trigger the changed API pathno 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

<!-- 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. `N/A — no user-visible behaviour change.` is the only empty value. 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
23 changes: 19 additions & 4 deletions journeys/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ stay diffable. Only the platform mechanics differ — `adb` becomes `xcodebuildm
`testTag`s become iOS `accessibilityIdentifier`s (the vocabulary is shared; see [Identifiers](#identifiers)).
iOS-only suites are marked in the [Suites](#suites) table.

**Journeys are not a QA gate.** They are agent-evaluated and non-deterministic, nothing runs them in
CI, and there is no runner wired up for them yet — `ai-device-tests.yml` runs `TrezorBridgeDashboardUITests`
and does not read `journeys/`. Treat a journey as a well-written description of a flow, not as an
authority on what the app owes you.
**Journeys are the QA contract for a PR.** A PR with a user-visible change adds or updates the
journeys that prove it and lists them in its body, and reviewers drive the listed journeys on a
device instead of reading a prose walkthrough.

A journey that no longer matches the app is most likely **stale**, not evidence of a bug. The corpus
is new on iOS and has not been run end to end, so when the two disagree the first assumption should be
Expand Down Expand Up @@ -130,6 +129,22 @@ Known naming differences:
Everything else — `N0`–`N9`, `N000`, `NDecimal`, `NRemove`, `SpendingAmount*`, `SpendingAdvanced*`,
`External*`, `Hardware*`, `Widget*` — matches Android exactly.

## Capabilities

This table is the authority for what the journey environment provides: a step it covers belongs in a
journey, and a step it does not is a manual test in the PR body naming the missing capability.

| Capability | Provided by |
| --- | --- |
| On-chain funds and blocks on regtest | `../bitkit-android/lsp` deposit and mine, borrowed from the sibling Android checkout until #694 lands an iOS copy — [Backend preconditions](#backend-preconditions) |
| Lightning channels, CJIT orders and quoted maxima | the LSP the `E2E_BUILD` app targets, plus its node as an external LN peer — [Backend preconditions](#backend-preconditions), [amount-limits](amount-limits/README.md) |
| A hardware wallet to pair, watch and sign with | the deterministic Trezor emulator from `bitkit-docker`, reached through Trezor Bridge on the host; the simulator has no Bluetooth LE and iOS cannot do WebUSB, so BLE pairing and USB are not covered — [hardware-wallet](hardware-wallet/README.md) |
| Push notifications to a backgrounded or killed app | a real APNs round trip on an attached physical device, never the simulator, read back from Notification Center — [cjit-notifications](cjit-notifications/README.md) |
| The OS notification-permission dialog | the one-shot `UNUserNotificationCenter` alert, reset with `xcrun simctl uninstall <device> to.bitkit` and a rebuild — [notification-permission](notification-permission/README.md) |
| An incoming Payment Request from a linked issuer | the fixture issuer, saved as a contact and linked on receiver path `bitkit/server` — [payment-requests](payment-requests/README.md) |
| A Pubky identity and a two-wallet marketplace purchase | a Bitkit-generated Pubky profile, plus the integration fixture runtime: Pubky testnet, Paykit Server, regtest bitcoind and Fulcrum — [pubky-auth](pubky-auth/README.md), [pubky-marketplace](pubky-marketplace/README.md) |
| Deep links handed to the app | `xcrun simctl openurl <device> "<uri>"`; only `bitkit://pubky-auth/setup`, web URLs, Pubky callbacks and payment URIs route — there is no screen or sheet router — [pubky-auth](pubky-auth/README.md), [Not ported](#not-ported) |

## Suites

| Suite | Journeys | Notes |
Expand Down
Loading