test: port Android journeys to iOS - #691
Conversation
Ports the journey concept from bitkit-android: XML behaviour specs evaluated by an agent driving a simulator through the XcodeBuildMCP CLI. Adds the runner guide, the Android/iOS identifier mapping, and the AGENTS.md rule that an Android port carries its journeys with it. Adds the accessibility identifiers the ported journeys assert on, matching the Android testTag vocabulary: - Screen containers: SendAmount, SpendingAmount, SpendingAdvanced, ExternalAmount, ReceiveCjitAmount, ReceiveCjitConfirm, ReceiveCjitLiquidity - Fields: SpendingAmountNumberField, SpendingAmountAvailable, ExternalAmountAvailable, ReceiveCjitAmountNumberField - Controls: Send, Receive, ReceiveCjitAmountContinue, NotificationsOpenSystemSettings - Over-max toasts: SpendingAmountExceededToast, SpendingAdvancedExceededToast, ExternalAmountExceededToast (Send already had one) - The three "Enable background setup" switches, which shared one binding and no ids Widget list tiles are onTapGesture views, so the runtime UI snapshot did not expose them as tappable targets. They now combine their children and report the button trait, making each tile one addressable element; without the combine the identifier fanned out to every child label. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four journeys covering the number pad cap on every amount-entry screen: Send, Transfer to Spending, Receiving capacity, and External node funding. Walked end to end on an iPhone 17 simulator. The two screens cap differently and the journeys assert "does not exceed the stated maximum" rather than a value: Send keeps the largest all-9s value under the cap (99 999 against 297 393), while Transfer to Spending clamps to the max exactly (296 522). Delete works at the cap on both and Continue stays enabled throughout. Entry is via the Savings card, not the Spending card as on Android: the iOS Spending screen only offers "Transfer From Savings" while the spending balance is zero, and shows "Transfer To Savings" once a balance exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two journeys covering the widgets intro on first run and the add-widget flow. Both walked end to end on an iPhone 17 simulator with no backend running. The home page "Add Widget" button sits below the fold and needs a scroll before it can be tapped, which Android does not; the sheet scroll Android needs is conditional on iOS since all six tiles fit on an iPhone 17. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three journeys covering the "Enable background setup" toggle on the Receive CJIT confirm, Receive CJIT liquidity and Transfer Spending confirm screens. Android's fourth journey does not apply: toggling off on iOS only unregisters push and stays in place, it does not deep link into system settings. It is replaced by toggle-off-and-system-settings-route.xml, which asserts that behaviour and then covers the real iOS route into Settings, the button on Settings > Notifications. That screen's identifiers were verified on simulator. All three toggles bind to one setting, so they must be run one per app state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three journeys covering single delivery of a CJIT payment notification and the rule that a non-CJIT channel opening is never reported as a received payment. Adapted rather than transcribed. NotificationService maps a Blocktank push type to fixed copy and never formats an amount, so Android's thousands-separator assertions have no iOS counterpart and are dropped. The duplicate-notification and wrong-type assertions port directly against the iOS copy: cjitPaymentArrived renders "Incoming Payment", orderPaymentConfirmed renders "Spending Balance Ready". Android's foreground-service journey becomes an app-backgrounded one. These need a physical device: the extension has to decrypt a real Blocktank payload delivered over APNs. Inspection is via the extension's os_log output, unlike the main app which writes log files into the app group. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fifteen journeys covering pairing, the home tile and detail screen, the Hardware Wallets settings surface, passphrase wallets, wallet-scoped activity, and the watch-only Transfer To Spending flow. Android's usb-reconnect becomes reconnect.xml: iOS cannot do WebUSB and has no USB_DEVICE_ATTACHED intent to inject, so the disconnect and reconnect chain runs through Trezor Bridge and the dev Trezor screen instead. Verified on simulator as far as one allows. The simulator has no Bluetooth LE, so every journey that pairs or reconnects needs the Bridge build or a physical device; the read-only journeys run on a plain simulator build. Two Android assumptions were corrected: iOS settings has no "Payments" section, and the connect intro headline renders uppercase. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#686 settles both transfer maximums and adds to SpendingAdvancedView the snap-to-max that SpendingAmount.onMaxExceeded() already performs on master. The existing assertion holds either way, but the disabled pad while settling, the snap down to the settled value, and the clamp when Max is tapped early are not covered yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Greptile SummaryThe PR ports Android journey specifications to iOS and adds the accessibility identifiers needed to evaluate them.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| Bitkit/Views/Widgets/WidgetsListSheetView.swift | Makes widget tiles unified accessibility elements and now applies the button trait only while tile activation is enabled. |
| journeys/cjit-notifications/README.md | Documents the physical-device and APNs requirements and correctly states that the simulator-only workflow cannot currently host this suite. |
| AGENTS.md | Adds XcodeBuildMCP usage and conventions for running and porting cross-platform journeys. |
| journeys/README.md | Introduces the shared guidance and inventory for agent-evaluated iOS journeys. |
| Bitkit/Views/Transfer/SpendingAmount.swift | Adds journey-facing accessibility identifiers to the transfer amount screen and its over-limit toast. |
Reviews (2): Last reviewed commit: "fix: address review on disabled widget t..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A journey is a shared spec, so a behaviour meant to match Android can be checked by running the same file on both sides. Documents the android CLI equivalents of the XcodeBuildMCP commands, and what to do when the two platforms disagree: record an intentional difference on both sides, or report a divergence as the bug it is, rather than rewriting the journey to match current behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Disabled widget tiles announced themselves as actionable buttons while their tap handler returns early, so VoiceOver offered a double-tap that did nothing. The button trait is now only added when the tile is enabled; the tile stays present by identifier, so journeys can still assert on it, but it is no longer an actionable target and WidgetEnableInSettings is the only control offered. The cjit-notifications suite requires a physical device but the README pointed at ai-device-tests.yml, which runs on the self-hosted macOS runner yet builds for platform=iOS Simulator, and gave a simctl log-stream command and a simulator snapshot step. Corrected to device commands, and the missing CI home is now stated rather than implied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
This will help us review and test PRs faster and also reproduce bugs more reliably |
Ran the documented commands against a live emulator. Two were wrong: `android emulator start` takes a required AVD name rather than defaulting, and `android layout` emits hyphenated JSON keys (`resource-id`, `content-desc`) rather than camelCase. Also records that the Settings screen agrees on nine identifiers across the two platforms, so a comparison run is mostly signal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Running the same Settings screen on both platforms surfaced two rows that resolved to the identifier "Value" on iOS, where Android has LanguageSettings and TransactionSpeedSettings. "Value" is the hardcoded identifier on the row's right-hand label in SettingsRow, so several rows were competing for it and neither row could be targeted. The Language row had no identifier at all. Transaction Speed had one, but `.accessibilityElement(children: .contain)` stopped the NavigationLink being the addressable element and let the inner value label surface instead — every sibling row that works uses a plain identifier with no container modifier. Dropped the modifier on both and added the missing identifier. Verified on simulator: the rows now resolve as LanguageSettings and TransactionSpeedSettings, matching Android. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paid a Lightning invoice generated on the iOS simulator from the Android emulator, end to end against the staging regtest backend. Two things worth writing down: `adb shell input text` drops characters — it lost 54 of a 397-character invoice — and `adb shell cmd clipboard` is not implemented on the emulator image. The reliable route is handing the app a lightning: URI via `am start`, which also skips the recipient screen. Also corrects the identifier table: Android emits both AvailableAmount and available_balance on the send amount screen, not only the latter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
The hardware-wallet activity-tags journey binds ActivityTag and ActivityTags to the wrong controls, so an identifier-first run can treat the Add Tag button as the chip and then miss the All Activity filter. This is iOS-only: Android's original journey does not attach those ids to those steps, and the shared app vocabulary already matches on both platforms.
Traced against ActivityItemView and ActivityListFilter: ActivityTag is the Add Tag button and ActivityTags is the detail screen's chip list, so the chip assertion could pass merely because the Add Tag button was still on screen, and the All Activity filter lookup would miss entirely — that control is TagsPrompt, same as Android. Add Tag now names ActivityTag, the chip assertion names ActivityTags, and the filter names TagsPrompt. Suite README updated to spell out the distinction. Reported by @ovitrif in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ben-kaufman
left a comment
ben-kaufman
left a comment
There was a problem hiding this comment.
The Swift changes build cleanly, but several journey definitions cannot run or validate the intended behavior as written. These harness issues should be corrected before merge so agents do not get deterministic false failures or false passes.
| `<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 agent-evaluated and non-deterministic: run them from the manual `ai-device-tests` |
There was a problem hiding this comment.
The only workflow_dispatch option in .github/workflows/ai-device-tests.yml is trezor-emu, and that job runs TrezorBridgeDashboardUITests. It never reads or evaluates journeys/. Could we either add a journey option or job, or point this instruction at the runner that actually evaluates these XML specs?
| maximum and the journeys pass for the wrong reason: | ||
|
|
||
| ```bash | ||
| ./lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":100000}' |
There was a problem hiding this comment.
bitkit-docker on main does not contain an ./lsp executable, so these commands fail with no such file and the backend-dependent journeys cannot satisfy their preconditions. Could we use a checked-in helper or document the actual curl or docker compose commands?
| (which writes log files into the app group). Stream it from the Mac the device is attached to, | ||
| while the push lands: | ||
| ```bash | ||
| log stream --device --predicate 'eventMessage CONTAINS "🔔"' |
There was a problem hiding this comment.
/usr/bin/log stream on macOS does not accept --device, so this exits before the push arrives. Configured notification is also logged at .info, so the replacement needs to include info messages. Please replace this with a tested attached-device command or use Notification Center as the supported assertion path.
| <action>Navigate to the wallet home screen and verify two hardware wallet tiles are shown beneath the SAVINGS and SPENDING tiles, one of them labelled "Hidden Trezor"</action> | ||
| <action>Verify the headline total balance is at least the sum of both hardware tile balances</action> | ||
| <action>Tap the "Hidden Trezor" tile and verify its hardware wallet detail screen opens (id "HardwareWalletScreen") titled "Hidden Trezor"</action> | ||
| <action>Resolve the app group container and grep it for the passphrase: GROUP=$(xcrun simctl get_app_container booted to.bitkit groups | awk '{print $2}'); grep -rl "bitkit-hidden" "$GROUP" || echo NO_PASSPHRASE_LEAK</action> |
There was a problem hiding this comment.
grep ... || echo NO_PASSPHRASE_LEAK prints success for every nonzero status, including a missing container, unreadable files, or an unresolved simulator. That means this security check can pass without scanning anything. Could we resolve the intended simulator explicitly and only treat grep status 1 as success, while failing on other statuses? The same issue appears in passphrase-transfer-to-spending.xml.
| <action>Tap the delete (trash) icon on the "Hidden Funds" row, and confirm "Remove" in the dialog (id "RemoveHwWalletDialog")</action> | ||
| <action>Verify the Hardware Wallets screen now lists exactly one row, the standard wallet, still showing its balance</action> | ||
| <action>Navigate to the wallet home screen and verify a single hardware wallet tile remains with a non-zero balance</action> | ||
| <action>Count the stored Trezor credentials: GROUP=$(xcrun simctl get_app_container booted to.bitkit groups | awk '{print $2}'); ls "$GROUP"/trezor-thp-credentials/ 2>/dev/null | wc -l</action> |
There was a problem hiding this comment.
TrezorCredentialStorage stores these credentials as Keychain generic-password items under service to.bitkit.trezor.thp. It does not create $GROUP/trezor-thp-credentials, so this command reports zero and the next assertion fails. Could this validate the remaining identity through observable app behavior or a test-only Keychain query?
| <action>Verify an in-app "Spending balance ready" toast appears (id "SpendingBalanceReadyToast")</action> | ||
| <action>Open Notification Center and inspect the Bitkit entries</action> | ||
| <action>Verify NO "Incoming Payment" notification was posted for this channel opening</action> | ||
| <action>Verify any notification posted for it uses the "Spending Balance Ready" copy, and that the streamed log shows type=orderPaymentConfirmed rather than cjitPaymentArrived</action> |
There was a problem hiding this comment.
This log cannot be produced in the stated foreground scenario. LightningService holds the .lightning process lock while its node is running, and NotificationService.didReceive returns on that lock before decrypting or logging the configured type. Please either background the app for the extension assertion or remove this log assertion and validate the foreground toast plus absence of an incoming-payment notification.
| prompts once per address type and the call blocks until it is acknowledged. Either tap the | ||
| emulator UI or run: ../bitkit-docker/scripts/trezor-emulator send-json '{"type":"emulator-press-yes","id":1}' | ||
| </action> | ||
| <action>Verify an error toast reports the passphrase wallet is already being watched, and that the sheet stays on the Passphrase step (id "HardwareWalletPassphraseScreen") with an empty input</action> |
There was a problem hiding this comment.
onPassphraseFailed clears the input and assigns errorMessage, and HwPassphraseView renders it inline with identifier HwPassphraseError. No toast is emitted here. Could the journey assert that inline error instead?
ovitrif
left a comment
ovitrif
left a comment
There was a problem hiding this comment.
@jvsena42, I rechecked the follow-up to my identifier comment. ActivityTag, ActivityTags, and TagsPrompt now point to the intended controls and preserve Android parity, so that issue is resolved and I’m approving.
I think two pieces of guidance added here should be reconsidered before they turn every journey detail into a merge requirement. Android does not have the new iOS journeys/README.md rule that a journey is the source of truth and any disagreement means the app failed. The referenced ai-device-tests workflow is also something I added earlier; it only runs TrezorBridgeDashboardUITests and is not a journey runner. Rather than expanding that workflow, I think this PR should remove or reword the claim that journeys run there and describe journeys more modestly as agent-driven developer-assistance specs, not QA gates.
@ben-kaufman surfaced useful details, and I appreciate the care behind them, but I would triage them against Android before making them blockers: fix iOS-specific mistakes introduced by the port, preserve or document inherited Android limitations, and leave broader runner or hardening work for separate follow-ups. That keeps this PR focused on Android parity without dismissing the review.
|
Draft for check all comments |
3 participants
This PR:
bitkit-android/journeystojourneys/.androidCLIs for agents, and makes porting journeys part of porting an Android feature.Description
File names, journey names and action prose are kept identical to the Android originals so the two platforms stay diffable. Only mechanics change:
adbbecomesxcodebuildmcp, testTags become accessibility identifiers.Adapted where iOS genuinely differs, each noted in its own description and suite README:
Most identifiers already matched, including the whole number pad. The gaps were screen containers, a few fields, the three background-setup switches which shared one binding and had none, and the over-max toasts on the transfer screens.
One behavioural change: widget list tiles are tap-gesture views, so the runtime snapshot never listed them as tappable. They now combine their children and carry the button trait, which also makes a tile read as one item under VoiceOver instead of separate fragments. No changelog fragment, since nothing else here is user-facing — but that part is worth a second opinion.
Trying it out
Because a journey is a shared spec, the same file can be run on Android too —
AGENTS.mdhas theandroidCLI equivalents, and says to record a disagreement between the platforms rather than rewrite the journey to match.The widgets journeys need no backend and are the quickest way to see the loop work. The rest need regtest from a sibling
bitkit-dockercheckout; the hardware wallet ones also need the Trezor emulator and the Bridge build, since the simulator has no Bluetooth LE. Preconditions are in each suite README.Linked Issues/Tasks
Screenshot / Video
No visual changes. The only UI-affecting edit is accessibility metadata on the widget list tiles.
QA Notes
Manual Tests
You should not have to name a file. Ask for the flow and let the agent pick the journey:
test the widgets flowtest the send flowtest the changes from this branchopen a channeldo a cross-platform paymentcheck whether any journey has drifted from the codeThe last one is the interesting one: a journey that no longer matches the app should come back as a failure, not be worked around.
Automated Checks
ai-device-testsworkflow rather than a blocking gate.send-amount-over-balanceandtransfer-spending-over-maxwalked end to end, and every added identifier asserted present with a deliberately bogus identifier as a control.swiftformat --lintreports only two pre-existing failures, in files this branch does not touch.