Skip to content

Refine mobile pairing confirmation - #6018

Merged
wesbillman merged 3 commits into
mainfrom
kennylopez-mobile-pairing-confirmation
Aug 18, 2026
Merged

Refine mobile pairing confirmation#6018
wesbillman merged 3 commits into
mainfrom
kennylopez-mobile-pairing-confirmation

Conversation

@klopez4212

Copy link
Copy Markdown
Contributor

Summary

  • align the mobile security-code confirmation screen with the Add Community visual system while keeping the SAS verification flow unchanged
  • use “Confirm desktop code” and a 15% primary-color outline on each digit box
  • support a gitignored, debug-only Android app name and application ID override for side-by-side device testing; release and profile identities remain unchanged

Validation

  • flutter test test/features/pairing/pairing_page_test.dart (21 tests)
  • full mobile pre-push suite (1,469 tests)
  • focused Flutter analysis and formatting
  • mobile worktree identity contract checks

Signed-off-by: kenny lopez <klopez4212@gmail.com>
klopez4212 added a commit that referenced this pull request Aug 16, 2026
@klopez4212

Copy link
Copy Markdown
Contributor Author

🤖 ### Mobile pairing confirmation

Deterministic placeholder code (000 000); no live pairing data or credentials.

confirm-desktop-code

@klopez4212
klopez4212 marked this pull request as ready for review August 16, 2026 08:57
@klopez4212
klopez4212 requested a review from a team as a code owner August 16, 2026 08:57

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0bd6baa0af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread mobile/lib/features/pairing/pairing_page.dart

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Team review: changes requested at 0bd6baa0af3e1350466a4252bde17c79d653cad2

Two material regressions remain in the high-risk identity-transfer confirmation:

  1. [P2] Restore the ordinary pairing trust/initiation warning. mobile/lib/features/pairing/pairing_page.dart:244-251 now tells the target only that its identity will transfer. The base explicitly warned users to confirm only a pairing they initiated from desktop, while the new test at mobile/test/features/pairing/pairing_page_test.dart:517-520 asserts that warning is absent. SAS authenticates the session endpoints; it does not establish that the other endpoint is the device the user intended to authorize. This conflicts with the repository's own NIP-AB UX/security contract (crates/buzz-core/src/pairing/NIP-AB.md:275-277,525-551). Restore direction-specific initiation/trust copy and assert it before the confirm action.

  2. [P2] Use an error treatment with readable contrast on the fixed onboarding gradient. The SAS screen now always uses the light chartreuse→pale-blue background (pairing_page.dart:109-143), including under dark theme, but reachable confirmation errors still render in context.colors.error (:319-326). Dark-theme error #ED8796 (mobile/lib/shared/theme/color_scheme.dart:39-56) measures only 1.60:1 against #D7D72E and 1.96:1 against #D7E7F6, far below 4.5:1 for bodySmall. Use a surface-specific accessible color/container and add actual error-state plus large-text coverage.

The added tests strongly pin cosmetic implementation values but do not exercise the semantic tree, large text, or the reachable confirmed/error layouts. Add contract-shaped accessibility coverage rather than only pixel-recipe assertions.

Verification: live PR head rechecked equal; clean detached tree; git diff --check clean; scripts/test-mobile-worktree-overrides.sh passed all contract checks. GitHub Mobile and all required checks are green. A focused local Flutter run produced no output and timed out after 10 minutes under shared Flutter/Gradle contention, so I do not claim a local Flutter pass. No real-device VoiceOver/TalkBack journey was supplied or run. These source-proven findings do not depend on those missing runs.

@brow

brow commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

🤖 Review note on the SAS confirmation screen at 0bd6baa0af3e1350466a4252bde17c79d653cad2. One blocking finding, plus two things I looked at and am not asking you to change.

Blocking: the error text on the SAS screen is below the accessibility bar this repo sets for itself, and it was above it before this change.

The SAS branch now paints every theme on the fixed onboarding gradient (_onboardingChartreuse #D7D72E to _onboardingShellBottom #D7E7F6 in pairing_page.dart), while the error line in _SasVerificationView still takes its colour from the theme (context.colors.error) at 14 px bodySmall.

Measured contrast, computed from the checked-in colour constants and swept across the whole gradient in 101 steps:

theme error colour contrast range on the new gradient required
dark #ED8796 1.604:1 to 1.957:1 4.5:1
light #D20F39 3.529:1 to 4.305:1 4.5:1

At the merge base d8281b9c93395f15d55091b131bb2747a0a3da8a the same text sat on context.colors.surface and passed in both themes: 5.964:1 dark and 5.430:1 light. So this is a regression introduced here, not inherited debt. Light theme fails too, which has not been raised before.

The state is reachable rather than theoretical. pairing_provider.dart sets errorMessage while status stays confirmingSas on the biometric failure and cancellation paths, and the existing unchanged test pairing_provider_test.dart "unenrolled biometrics prevent a protected import" asserts exactly that pair of conditions. PairingPage keeps the gradient in that state and passes the error into _SasVerificationView. Dark theme is selectable in settings and this screen is reached from the app shell, the community sheet, and the invite join sheet.

The bar is the repo's own: VISION.md line 206 reads | Accessibility | WCAG 2.1 AA minimum |. It is the only WCAG statement in the repo docs.

Three independent computations of these numbers agree, and the instrument was calibrated first against known values (black on white 21.00, #767676 on white 4.54, identical colours 1.00). I am deliberately not proposing a specific treatment here, because any fix is a claim that needs its own measurement before it goes in a review comment.

One product point, on the record. The codex thread on this exact construct was declined citing the owner's UI-preservation direction, in a reply signed "- Fast Fizz". We read that direction as covering the visual design of the surface, not the foreground text contrast, because the two were compatible at the merge base and only became incompatible with this change. If the direction is in fact intended to cover foreground contrast as well, say so here and we will record this as accepted product intent rather than a blocker.

Not asking you to change, recorded for completeness:

  1. The ordinary pairing path lost the direction-specific warning "Only confirm if you initiated this pairing from your desktop." The recovery path kept its equivalent. I read NIP-AB.md at this head: its requirement is that the prompt state what is being authorized and offer a clear deny, which the new copy does, and its own example wording carries no initiation clause. So this is a strong ask, not a gate. Worth noting that the new assertion find.textContaining('Only confirm if you started this pairing.'), findsNothing pins a literal that never existed anywhere in the tree, so it is near-vacuous, and adding an initiation warning back leaves the 21 test pairing page suite green.
  2. AppOverrides.properties is validated during global Gradle configuration, so a malformed ignored file fails configuration for release-oriented invocations too. Release and profile application id and app name were unchanged in every arm we ran, including absent, empty, garbage, and hostile values, and the base already throws at the same scope for a malformed worktree.properties. Pre-existing hazard shape, not chargeable here.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Team re-review: changes requested at 0bd6baa0af3e1350466a4252bde17c79d653cad2

Both independent lanes reproduced the two unresolved P2 regressions at this exact head:

  1. [P2] Restore direction-specific trust/intent guidance for ordinary pairing. mobile/lib/features/pairing/pairing_page.dart:244-247 warns recovery users to continue only if they started recovery, but the ordinary desktop→mobile path says only that the identity transfers “to this device.” The test at mobile/test/features/pairing/pairing_page_test.dart:517-520 now asserts the absence of a differently worded warning and does not positively protect the required intent cue. NIP-AB requires an unambiguous authorization prompt and verification against the intended physical device (crates/buzz-core/src/pairing/NIP-AB.md:275-277,523-529,543-551). SAS authenticates the session endpoints; it does not prove the user intended to authorize this desktop. Restore concise ordinary-path copy such as “Only continue if you started this pairing from your desktop,” and positively assert it before confirmation.

  2. [P2] Use a gradient-safe accessible error treatment. The confirming-SAS page always uses the fixed light chartreuse→pale-blue gradient (mobile/lib/features/pairing/pairing_page.dart:109-143; pairing_page/onboarding_background.dart:13-16), while reachable errors remain 14px context.colors.error text (pairing_page.dart:319-326). From the checked-in colors, dark-theme #ED8796 measures only 1.60:1–1.96:1 and light-theme #D20F39 3.53:1–4.30:1 across the gradient endpoints, all below the 4.5:1 requirement in VISION.md:206. These errors are reachable after changed-community/reauthorization and biometric failures (pairing_provider.dart:187-255). Use a surface-specific foreground/container verified across the full gradient and both themes, and add reachable error-state contrast/layout coverage.

No additional protocol/state or build-identity blocker was found. Transcript verification still gates import, payloads remain buffered until confirmation, denial clears state, and stale async completion is fenced (pairing_provider.dart:303-332,558-665,694-759). Android overrides are validated and consumed only by debug; release/profile retain production defaults (mobile/android/app/build.gradle.kts:37-67,106-146). The grouped SAS semantics and confirm/cancel ordering are improvements. A 320×568 large-text overflow in the confirmed row was reproduced at the base and is therefore inherited debt, not charged to this PR.

Exact-head evidence: clean tree; git diff --check passed; scripts/test-mobile-worktree-overrides.sh passed; focused pairing tests passed 21/21; independent full just mobile-test passed 1,469 tests; just mobile-check passed with analysis clean and 409 files unchanged; GitHub Mobile and applicable checks are green. The supplied screenshot covers only the default light happy path and is not an exact-head native accessibility receipt; no physical-device VoiceOver/TalkBack journey was run. Those gaps do not disprove the source- and contrast-proven blockers.

Any new head requires a fresh review.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Royal Court verdict: CHANGES REQUESTED

Reviewed exact head 0bd6baa0af3e1350466a4252bde17c79d653cad2 against base d8281b9c93395f15d55091b131bb2747a0a3da8a. All four lanes independently converged on the same two material regressions in this identity-transfer confirmation screen.

  1. [P2] Restore direction-specific initiation/trust guidance for ordinary pairing. mobile/lib/features/pairing/pairing_page.dart:244-247 retains “Only continue if you started this recovery” for recovery, but ordinary desktop-to-mobile pairing now says only that the identity transfers to this device. The base warned users to confirm only a pairing they initiated from desktop. NIP-AB requires an unambiguous authorization prompt and describes the defense as user verification against the intended physical device (crates/buzz-core/src/pairing/NIP-AB.md:275-277,523-551). SAS authenticates the session endpoints; it does not establish user intent. Restore concise ordinary-path guidance such as “Only continue if you started this pairing from your desktop,” and positively assert it before confirmation. The new negative assertion protecting the warning’s absence should be replaced with contract-shaped coverage.

  2. [P2] Use a gradient-safe accessible treatment for reachable errors. The SAS branch now always paints the fixed light chartreuse-to-pale-blue onboarding gradient (pairing_page.dart:109-143), while errors remain ambient context.colors.error at bodySmall (:319-326). Against the checked-in gradient endpoints, dark-theme #ED8796 measures 1.60:1–1.96:1 and light-theme #D20F39 3.53:1–4.30:1, below the repository’s WCAG 2.1 AA minimum (VISION.md:206). These states are reachable while still on this screen after changed-community and device-auth/biometric failures (pairing_provider.dart:187-255). Use a surface-specific foreground/container verified across the gradient and both themes, then add reachable error-state contrast/layout coverage.

No additional protocol-state or Android override blocker was found. Transcript-hash verification still gates transfer, payloads remain buffered until user confirmation, denial cleans up session state, stale async completion is fenced, and Android overrides are validated and consumed only by debug. Grouped SAS semantics and confirm/cancel ordering are improvements.

Tests changed substantially because behavior changed: the pairing widget suite adds and rewrites visual, layout, navigation, copy, and small-screen assertions. Please specifically review the assertion that currently requires the ordinary trust warning to be absent, and add error/accessibility coverage shaped around the reachable failure state rather than only cosmetic implementation values.

Evidence at this head: git diff --check is clean; GitHub Mobile and all applicable checks pass; the PR is conflict-free. Existing exact-head independent validation reports focused pairing tests 21/21, full mobile tests 1,469, and clean mobile analysis. I did not duplicate CI-equivalent suites locally. No physical-device VoiceOver/TalkBack journey was run. Any new head requires re-review.

wesbillman and others added 2 commits August 18, 2026 14:17
Restore the ordinary pairing intent cue and use a fixed-surface error color
that meets WCAG AA across the onboarding gradient. Cover the reachable
error state in both themes.

Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Bring the pairing confirmation branch current with main after the branch-skew
check detected overlapping mobile documentation changes.

Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
Signed-off-by: Wes <wesbillman@users.noreply.github.com>

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Re-reviewed exact head b1532f6 after the two requested fixes. Ordinary desktop-to-mobile pairing again gives explicit initiation/trust guidance, and reachable SAS errors now use a fixed light-surface color with WCAG AA contrast across both gradient endpoints. Tests positively cover the trust copy and the reachable error state in light and dark themes.

Focused pairing tests pass (22/22), flutter analyze is clean, and the mandatory pre-push hook passed all lanes on this exact head. No remaining material findings.

@wesbillman
wesbillman merged commit 40f1dac into main Aug 18, 2026
25 checks passed
@wesbillman
wesbillman deleted the kennylopez-mobile-pairing-confirmation branch August 18, 2026 21:06
wpfleger96 pushed a commit that referenced this pull request Aug 18, 2026
…c-agent-commit-identity

* origin/main:
  Refine mobile pairing confirmation (#6018)
  chore(scripts): add buzz-adopt-prod-agents.sh (#6250)

Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
jedwards27 pushed a commit to jedwards27/buzz that referenced this pull request Aug 18, 2026
* origin/main: (43 commits)
  perf(desktop): parallelize relay agent directory rebuild (block#6258)
  Refine the mobile emoji picker (block#5853)
  fix(desktop): exclude archived agents from nest, order regeneration (block#5905)
  Add font size and conversation density preferences (block#5644)
  fix(desktop): emit camelCase config-write payload fields (block#6062)
  fix(desktop): downscale large avatars for agent-share PNG body (block#6260)
  fix(desktop): preserve early relay auth challenges (block#3320)
  Polish mobile message actions (block#5873)
  Refine mobile pairing confirmation (block#6018)
  chore(scripts): add buzz-adopt-prod-agents.sh (block#6250)
  feat(managed-agents): close five Claude Code agent-config gaps (block#4557)
  chore(hooks): keep mobile analysis out of pre-commit (block#6236)
  fix(shared-ui): delay hover disclosures by default (block#5821)
  fix(desktop-chrome): preserve balanced layout when sidebar collapses (block#6000)
  Polish mobile timeline navigation (block#5874)
  chore(release): release Buzz Desktop version 0.5.17 (block#6234)
  fix(prompt): simplify pickup follow-through (block#6186)
  fix(mcp): scope todo usage (block#6216)
  fix(desktop): bound remote agent mention authorization (block#6224)
  fix: bump h2 for RUSTSEC-2026-0258 (block#6222)
  ...

Signed-off-by: Princess Donut <3cb959c7eb65d61f634e61df318e450f18f82fa0e01849e7010b82666ead0587@buzz.block.builderlab.xyz>

# Conflicts:
#	desktop/src/main.tsx
#	mobile/ios/Podfile.lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants