Skip to content

fix(ios): defer QR pairing after scanner dismissal#99572

Merged
steipete merged 42 commits into
openclaw:mainfrom
PollyBot13:fix-ios-qrscanner-dismissal
Jul 5, 2026
Merged

fix(ios): defer QR pairing after scanner dismissal#99572
steipete merged 42 commits into
openclaw:mainfrom
PollyBot13:fix-ios-qrscanner-dismissal

Conversation

@PollyBot13

@PollyBot13 PollyBot13 commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #99571.

Fixes an issue where iOS users scanning a gateway pairing QR could have the scanner dismiss at the wrong time, lose the trust/approval handoff, or crash while VisionKit was still stopping camera capture.

The affected surfaces are the iOS onboarding scanner, Gateway Settings scanner, app/open-url setup-link handling, and post-scan gateway approval handoff.

Why This Change Was Made

The scanner now stops VisionKit scanning, queues one typed QR/setup result, dismisses the scanner sheet, and processes pairing only from the sheet dismissal path after a short main-actor settling delay. The final branch also handles setup links delivered during app/onboarding launch by staging UIKit app-delegate URLs through the same gateway setup-link path used by SwiftUI .onOpenURL.

This keeps the scanner lifecycle separate from gateway trust UI and avoids re-entering the onboarding/settings presentation stack while VisionKit is still dismissing.

User Impact

iOS users can pair a gateway by scanning a QR code from onboarding or from Settings without losing the approval flow. After approval, the app reaches Chat and can send/receive messages immediately.

Evidence

Local validation:

  • git diff --check
  • pnpm native:i18n:check
  • Focused iOS source-guard tests: 35 passed
  • Maintainer exact-head simulator proof: 35 RootTabsSourceGuardTests passed on iPhone 17 / iOS 26.5, including callback-order guards for both scanner owners and stop-before-delivery.
  • Maintainer autoreview: clean after one accepted test-specificity finding was repaired.

AI-assisted: yes.

@openclaw-barnacle openclaw-barnacle Bot added app: ios App: ios size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jul 3, 2026
@PollyBot13
PollyBot13 force-pushed the fix-ios-qrscanner-dismissal branch from 827934a to d7700b8 Compare July 3, 2026 14:20
@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 5, 2026, 3:28 PM ET / 19:28 UTC.

Summary
The branch defers iOS QR/setup-link pairing until scanner sheet dismissal and broadens gateway route, auth, APNs, Watch, and session ownership handling with focused Swift and TypeScript coverage.

PR surface: Source +36, Tests +28, Docs +1, Other +7547. Total +7612 across 65 files.

Reproducibility: yes. The linked issue has physical iPhone current-main crash evidence, and current-main source still sends scanner results directly from the VisionKit callback into pairing work; I did not rerun the crash on a device.

Review metrics: 2 noteworthy metrics.

  • Current-head proof drift: 343 commits after video proof head. The visible physical iPhone proof was recorded on 1158373, while the live PR head is d3ae2b1 after later pairing/auth/watch changes.
  • Auth/state contract changes: 4 surfaces changed. Device auth storage, gateway credentials, connect options, and share relay config all affect upgrade/auth behavior beyond the scanner view.

Stored data model
Persistent data-model change detected: serialized state: apps/ios/Sources/Design/SettingsProTabActions.swift, serialized state: apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceAuthStore.swift, serialized state: apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99571
Summary: This PR is the open candidate fix for the canonical iOS QR scanner crash issue; the iOS onboarding polish PR is adjacent history, not the same root cause.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post fresh physical iPhone onboarding and Settings QR pairing proof on head d3ae2b1, with private QR/setup details redacted.
  • Remove the CHANGELOG.md entry and leave release-note context in the PR body or squash message.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The linked recordings are real physical-device proof, but they are for older head 1158373 and do not prove the current head d3ae2b1 after later pairing/auth/watch changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Physical iPhone camera proof is for head 1158373, while current head d3ae2b1 includes later pairing, auth, Watch, and merge commits; VisionKit teardown cannot be proven by simulator or CI alone.
  • [P1] The branch changes Keychain credential bundling, gateway-scoped device tokens, APNs approval identity, share relay auth, and Watch deferred payload ownership, so upgrade/auth behavior deserves maintainer-visible proof before merge.
  • [P1] The added CHANGELOG.md entry conflicts with the repository's release-owned changelog policy and should be removed from this PR.

Maintainer options:

  1. Require exact-head device proof (recommended)
    Get a fresh physical iPhone QR pairing run through onboarding and Settings on head d3ae2b1, then remove the release-owned changelog entry before merge.
  2. Accept stale camera proof
    A maintainer could intentionally accept the older physical proof plus current simulator/CI coverage, but that owns the risk that the latest head still races VisionKit teardown.
  3. Pause for a narrower branch
    If the auth/watch ownership expansion is too broad for this crash fix, pause this PR and split the QR dismissal repair from the gateway ownership hardening.

Next step before merge

  • [P1] This PR needs contributor or maintainer physical-device proof on the exact current head before merge; automation cannot supply that proof from the contributor's iPhone setup.

Security
Cleared: No concrete security or supply-chain issue was found; the auth and identity changes are compatibility-sensitive merge risks rather than suspicious code execution or secret-handling regressions.

Review findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:16
Review details

Best possible solution:

Land the scanner-dismissal and gateway-ownership fix after a fresh physical iPhone QR pairing proof on the exact current head and after removing the release-owned changelog edit.

Do we have a high-confidence way to reproduce the issue?

Yes. The linked issue has physical iPhone current-main crash evidence, and current-main source still sends scanner results directly from the VisionKit callback into pairing work; I did not rerun the crash on a device.

Is this the best way to solve the issue?

Yes, the scanner-dismissal owner path is the best fix shape for this UIKit/VisionKit race. The remaining gaps are exact-head physical proof and removal of the release-owned changelog edit, not a different runtime layer.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:16
    This PR adds an Unreleased changelog line, but AGENTS.md says CHANGELOG.md is release-owned and normal PRs should not edit it; release generation derives entries from merged PRs and direct commits. Please drop this line and keep the release-note context in the PR body or squash message.
    Confidence: 0.91

Overall correctness: patch is correct
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f141408e7749.

Label changes

Label changes:

  • add merge-risk: 🚨 compatibility: The branch changes persisted Keychain credential ownership, device-token scoping, and setup-link handling in ways that can affect existing upgraded installs.
  • add merge-risk: 🚨 auth-provider: The branch changes gateway device auth reuse, APNs approval gateway identity, and share/terminal token selection behavior.
  • add merge-risk: 🚨 availability: Without current-head physical iPhone proof, merging may leave the VisionKit scanner teardown crash or lost pairing handoff unresolved.
  • add proof: 🎥 video: Contributor real behavior proof includes video or recording evidence. The linked recordings are real physical-device proof, but they are for older head 1158373 and do not prove the current head d3ae2b1 after later pairing/auth/watch changes.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The linked recordings are real physical-device proof, but they are for older head 1158373 and do not prove the current head d3ae2b1 after later pairing/auth/watch changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The PR targets a real iOS crash and broken gateway pairing workflow affecting onboarding and Settings.
  • merge-risk: 🚨 availability: Without current-head physical iPhone proof, merging may leave the VisionKit scanner teardown crash or lost pairing handoff unresolved.
  • merge-risk: 🚨 compatibility: The branch changes persisted Keychain credential ownership, device-token scoping, and setup-link handling in ways that can affect existing upgraded installs.
  • merge-risk: 🚨 auth-provider: The branch changes gateway device auth reuse, APNs approval gateway identity, and share/terminal token selection behavior.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The linked recordings are real physical-device proof, but they are for older head 1158373 and do not prove the current head d3ae2b1 after later pairing/auth/watch changes. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • proof: 🎥 video: Contributor real behavior proof includes video or recording evidence. The linked recordings are real physical-device proof, but they are for older head 1158373 and do not prove the current head d3ae2b1 after later pairing/auth/watch changes.
Evidence reviewed

PR surface:

Source +36, Tests +28, Docs +1, Other +7547. Total +7612 across 65 files.

View PR surface stats
Area Files Added Removed Net
Source 7 52 16 +36
Tests 4 34 6 +28
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 53 9261 1714 +7547
Total 65 9348 1736 +7612

What I checked:

Likely related people:

  • steipete: Recent current-main history and this PR's repair comments touch QR pairing, Settings, gateway route ownership, and current-head preparation. (role: recent area contributor and current PR repair owner; confidence: high; commits: 862de9f1a1c2, 313560d5b98e, 13527dfc7f57; files: apps/ios/Sources/Onboarding/QRScannerView.swift, apps/ios/Sources/Design/SettingsProTabActions.swift, apps/ios/Sources/Gateway/GatewayConnectionController.swift)
  • BunsDev: Authored the gateway pairing setup hardening commit that added QR scan support from Settings and changed setup-code parsing and pairing surfaces. (role: feature-history contributor; confidence: high; commits: b2efd1964800, 2fd372836e52; files: apps/ios/Sources/Onboarding/QRScannerView.swift, apps/ios/Sources/Onboarding/OnboardingWizardView.swift, apps/ios/Sources/Design/SettingsProTabActions.swift)
  • mbelinky: Authored the earlier iOS onboarding and QR pairing stability flow and several adjacent pairing/reconnect hardening commits. (role: earlier QR pairing flow contributor; confidence: medium; commits: 130e59a9c096, 582870834308, 6e7f1a6a1bb7; files: apps/ios/Sources/Onboarding/QRScannerView.swift, apps/ios/Sources/Onboarding/OnboardingWizardView.swift, apps/ios/Sources/Gateway/GatewayConnectionController.swift)
  • Solvely-Colin: Authored the broad iOS Pro UI and gateway flows work that touched Settings, onboarding, gateway controller, and chat/session surfaces affected by this PR. (role: major adjacent iOS gateway-flow contributor; confidence: medium; commits: f6e51ff99af4; files: apps/ios/Sources/Design/SettingsProTab.swift, apps/ios/Sources/Design/SettingsProTabActions.swift, apps/ios/Sources/Onboarding/OnboardingWizardView.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (16 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-05T08:37:37.497Z sha 5d5da1f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T09:00:46.093Z sha f8a24ec :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T10:01:08.498Z sha 4d9a343 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T10:33:56.481Z sha f94c7ac :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T10:53:23.124Z sha 51aeb7e :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T12:09:59.355Z sha 13527df :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T16:29:50.327Z sha b41f5d7 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T16:35:29.146Z sha b41f5d7 :: needs real behavior proof before merge. :: none

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jul 3, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 3, 2026
Closed
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P1 High-priority user-facing bug, regression, or broken workflow. labels Jul 3, 2026
@PollyBot13

PollyBot13 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up pushed at 8ee81ce97c1fdeeb778d3d24fd99bd90057fadab for the first-scan trust prompt flash.

What changed:

  • Onboarding and Gateway Settings now process queued QR pairing results from the QR scanner sheet onDismiss callback instead of onChange(showQRScanner == false).
  • The existing queued result state and short post-dismiss delay remain in place, so trust/connection UI starts after the scanner sheet has actually been dismissed.
  • Added source-guard coverage to keep both scanner entry points on the sheet-level dismissal path and prevent the old showQRScanner trigger from coming back.
  • Synced native i18n inventory after Swift line movement.

Verification:

  • swiftformat --lint --config config/swiftformat apps/ios/Sources/Onboarding/OnboardingWizardView.swift apps/ios/Sources/Design/SettingsProTab.swift apps/ios/Tests/RootTabsSourceGuardTests.swift
  • swiftlint lint --config config/swiftlint.yml --strict -- apps/ios/Sources/Onboarding/OnboardingWizardView.swift apps/ios/Sources/Design/SettingsProTab.swift
  • pnpm native:i18n:check
  • xcodebuild test -project apps/ios/OpenClaw.xcodeproj -scheme OpenClaw -destination "platform=iOS Simulator,name=iPhone 17" -only-testing:OpenClawTests/RootTabsSourceGuardTests
  • git diff --check

No fresh physical-device proof in this update; this directly targets the presentation timing race Wouter saw after the crash fix.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 3, 2026
@steipete steipete self-assigned this Jul 3, 2026
@steipete
steipete force-pushed the fix-ios-qrscanner-dismissal branch from 8ee81ce to 93afb17 Compare July 3, 2026 18:49
@steipete

steipete commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Maintainer repair is pushed on exact head 93afb17f02e06ccc34bcd65e33a9b088a5e3564b.

The final branch now stops VisionKit scanning before requesting sheet dismissal, carries one typed pending result, and delivers pairing only from the sheet onDismiss path. Focused iPhone 17 simulator build/tests and fresh Codex autoreview are clean.

The remaining merge gate is physical-iPhone camera proof on this exact head. Please run a fresh QR through both:

  1. onboarding QR pairing;
  2. Gateway Settings QR pairing.

For each, confirm the scanner closes, pairing/trust UI proceeds, the gateway connects, and there is no crash or first-scan prompt flash. A redacted screen recording or screenshots plus device/iOS version are sufficient.

@PollyBot13

PollyBot13 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Physical-device follow-up on Polly's iPhone, exact PR head 2dbf3dfc08ae515b92e5f9fa2f002c49487a738e, locally proof-signed build 2026.6.11 / 63613.

Onboarding QR still shows the same first-scan behavior: the trust prompt appears briefly and then disappears before the user can act. The imported phone video from the previous build showed this is not just scanner-dismissal timing: the prompt appears after the scanner is gone, then the app transitions out of onboarding/main presentation and the prompt is lost. I tried keeping onboarding open for gatewaySetupRequestID while visible, but the physical-device behavior is unchanged.

Current contributor status: crash fix still looks separate, but the remaining onboarding trust-prompt flash is not solved by the dismissal delay or the first RootTabs onboarding guard. We'll continue diagnosis from the view/presentation lifecycle tomorrow before adding proof.

@steipete steipete mentioned this pull request Jul 4, 2026
Merged
@clawsweeper clawsweeper Bot removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 4, 2026
steipete added 26 commits July 5, 2026 12:59
@steipete
fix(ios): prevent stale gateway reconnects
3412941
@steipete
fix(ios): serialize gateway target handoff
c261158
@steipete
fix(ios): disable stale gateway relaunch route
c4033a0
@steipete
fix(ios): await staged bootstrap reset
7df76ea
@steipete
test(ios): bound gateway reset handoff
bdcef73
@steipete
fix(ios): preserve explicit gateway handoff
26b3ae2
@steipete
fix(ios): harden gateway lifecycle ownership
3c6bf6a
@steipete
chore(ios): sync native i18n inventory
7beb0cb
@steipete
test(ios): align gateway ownership assertions
f136f56
@steipete
refactor(ios): remove superseded gateway helpers
7d82769
@steipete
fix(ios): keep gateway auth route scoped
b25780c
@steipete
fix(ios): restore gateway target review state
d84ea72
@steipete
fix(protocol): refresh Swift plugin approval model
00a5682
@steipete
test(ios): isolate state directory overrides
ad4f7ed
@steipete
fix(ios): preserve watch alerts across gateway switches
13527df
@steipete
chore(ios): sync PR 99572 with main
b41f5d7
@steipete
chore(ios): sync PR 99572 with main
d497cff
@steipete
chore(ios): sync PR 99572 with latest main
ff0bb5a
@steipete
fix(ios): bind deferred work to gateway ownership
39c1d93
@steipete
Merge remote-tracking branch 'origin/main' into pr-99572-prep
4b2ef09
@steipete
docs(changelog): credit iOS gateway handoff fix
861361d
@steipete
Merge remote-tracking branch 'origin/main' into pr-99572-prep
4de32bf
@steipete
chore(i18n): sync native app inventory
8aca317
@steipete
test(ios): remove unused Watch approval hooks
81ab853
@steipete
Merge remote-tracking branch 'origin/main' into HEAD
7462532
@steipete
Merge remote-tracking branch 'origin/main' into HEAD
d3ae2b1
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 5, 2026
Open
@doschott doschott mentioned this pull request Jul 7, 2026
Open
@joshavant joshavant mentioned this pull request Jul 10, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

app: ios App: ios app: macos App: macos gateway Gateway runtime merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P1 High-priority user-facing bug, regression, or broken workflow. proof: 🎥 video Contributor real behavior proof includes video or recording evidence. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. scripts Repository scripts size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

iOS QR scanner can crash when pairing prompt appears during scanner dismissal

2 participants

@PollyBot13 @steipete