pneumo

M4: Pairing, onboarding and the app shell (ENG-253) #16

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-253-m4-pairing-onboarding-and-the-app-shell

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-253-m4-pairing-onboarding-and-the-app-shell

Linear: https://linear.app/engineereddev/issue/ENG-253

What's in this PR (all under apps/android/):

Theme: BrandColors.kt mapped to Material 3 light/dark (dynamic colour off) plus DESIGN.md typography; brand launcher and notification icons.
Navigation (ui/PneumoNavHost.kt): Pair → Confirm → Onboarding → main shell with bottom nav (Home, Devices, Settings). The start destination comes from SecureStore and DataStore state.
Pair: CameraX + ZXing core scanner (no Play Services), camera rationale and denied state, and "Paste link instead".
Deep link pneumo://pair → Confirm. Confirm shows the host (warns on non-HTTPS outside debug), claims, checks keyCheck against k, and saves MK + token. Mismatch follows §7.5 (delete own device, wipe); 410 → "This code has expired…".
Onboarding: notifications (API 33+) and the battery-optimization exemption, both skippable with a warning, shown once.
Placeholder Home/Devices/Settings; Settings Sign out (DELETE own device, wipe SecureStore, back to Pair).
AppContainer manual DI; PairingRepository, SessionPreferences, PairingViewModel.
A debug-only network security config allows cleartext to 10.0.2.2/localhost; release keeps usesCleartextTraffic="false".
36 new Roborazzi baselines, recorded on purpose (light + dark): Pair (idle, camera denied, paste, invalid), Confirm (https, non-https, claiming, retryable error, error), Onboarding (notifications, battery, done, warning), Shell (home, devices, settings, settings busy, sign-out error). The obsolete scaffold screen and its goldens were removed.

Verify (lead, rebased on 555a737):

mise run check:android → exit 0 (Spotless, detekt, Lint "no errors or warnings", JVM tests, Roborazzi verify)
mise run e2e:android → exit 0; forced rerun pixel6Api34DebugAndroidTest --rerun → BUILD SUCCESSFUL, "Running tests on devices: pixel6Api34"
testDebugUnitTest --rerun → 108 tests, 0 failures
Implementer's end-to-end check against a local server: deep-link pairing reached Home ("Paired with 10.0.2.2:8080"); a relaunch kept the session; sign-out removed the device on the server.

Deviations:

A narrowly scoped @SuppressLint("BatteryLife") on the spec-required battery-exemption helper. Lint flags the Play-policy API; Pneumo is sideloaded with no FCM. No project-wide disable or baseline entry.
The managed (ATD) image has no Settings/SystemUI, so the battery request exercised the "settings unavailable" warning path there. The real dialog and physical-camera scanning need a regular emulator or phone (M5 acceptance).

🤖 Generated with Claude Code

Commits 0

Could not load commits.

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, REQUEST_CHANGES #

The §7.5 core is correct: claim → /api/me → constant-time keyCheck compare (MessageDigest.isEqual), MK + token saved only on a match, mismatch deletes the device and wipes, 410 shows the exact expired message, and network errors retry without re-claiming. The debug-only cleartext config and the dependency hygiene are right. Two fixes and one DoD item before merge.

Findings #

FIX (often 4 × bad 4 = 16), MainActivity.kt:28 + PairingViewModel.kt:51: the pairing intent is re-handled on every onCreate. After process death the original VIEW intent comes back, so a user who paired by deep link lands on "Pair with ?" for a used code (→ 410, or Back to Pair while the session is still stored). On rotation or theme change, openLink("") shows a spurious "Invalid pairing link". On a cold start, any link can put a foreign server's Confirm screen in front of a paired user. Fix: handle the intent only when savedInstanceState == null and not launched from history, replace the intent, let restore win over a link when a session exists, and add a Robolectric recreate test.
FIX (often 10 × bad 1 = 10), MainShell.kt:67-68, PneumoNavHost.kt:40,49,65,72: window insets are mishandled. The main shell applies the status-bar inset twice (the header sits ~50 dp low), and Pair/Confirm/Onboarding pad outside their background, which leaves a band in the surface colour. Robolectric goldens can't show this (zero insets). Fix: drop the outer safeDrawingPadding(), and consume the Scaffold insets in MainShell.
Missing DoD item: the Pair screen's camera-granted state (224 dp framed preview box) has no @Preview or Roborazzi golden. 4–10. FILE (scores 2–4): sign-out needs a reachable server; misleading error for http:// in release; pendingClaim lost on process death; doubled warning copy; smoke tests bypass the intent filter; weak mismatch assertion; DESIGN.md focus ring and destructive icon missing. Recorded on ENG-275, code left alone.

Spec checks #

§7.5 OK; §6.7 filter and strict parser OK (re-handling is finding 1); §14.1 theme from generated BrandColors.kt, typography and radii per DESIGN.md, CameraX 1.5.1 + ZXing core 3.5.3, no GMS/ML Kit/Firebase in the release classpath, pinned versions, cleartext confined to debug 10.0.2.2/localhost; §14.3 copy and once-only onboarding OK; §6.9 OK.

DoD checks #

108 JVM tests (Turbine + Robolectric) pass; 36 goldens recorded on purpose; managed-device smoke extended; Spotless, detekt and Lint clean with one scoped @SuppressLint("BatteryLife"). Missing: the camera-granted preview (finding 3). The PR touches hot file app/build.gradle.kts for the new dependencies; no other in-flight issue holds it.

Tests run by the reviewer: mise run check:android exit 0 (108 tests); pixel6Api34DebugAndroidTest --rerun exit 0; release classpath checked for Play Services. → omp round 2 for findings 1–3.

v1 1w ago

demi.dev 1w ago

pneumo-reviewer: round 2, APPROVE #

Round 2 (600508c) fixes both FIX findings and adds the missing preview. The rebased round-1 commit d82ac13 is byte-identical to 338397e under apps/android, and the PR still touches only apps/android/.

Findings #

Round 1 FIX (score 16), deep-link replay and session override: resolved. MainActivity.onCreate handles the intent only when savedInstanceState == null and not launched from history, then swaps in an ACTION_MAIN copy with no data. openLink waits for restore and acts only when the destination is Pair, so a stored session wins on cold and warm starts. New Robolectric lifecycle tests and ViewModel precedence tests cover this.
Round 1 FIX (score 10), insets: resolved. The NavHost Surface uses colorScheme.background, the outer safeDrawingPadding() is gone, and MainShell consumes the Scaffold insets. In main-shell-r2.png and pair-r2.png the header sits at the same y, and the top inset is one uniform canvas colour.
Missing DoD (camera-granted preview): resolved. pair-camera-granted-{light,dark}.png recorded on purpose; the other 36 goldens are unchanged.
FILE (1), PairingIntentTest.kt:72-92: the saved-bundle test would also pass if the intent were replayed (ViewModel precedence masks it); use an unpaired fixture there.
FILE (1), PairingViewModel.kt:50: a pairing link opened while paired is ignored silently; a future Settings issue could say "Sign out first to pair again".
Round 1 FILE items 3–9: unchanged, recorded on ENG-275.

Spec / DoD #

§7.5, §6.7, §14.1, §14.3, §14.10 and §6.9 OK. 114 JVM/Robolectric tests, previews for every state, baselines recorded on purpose with the commit saying so, managed-device smoke present, Spotless/detekt/Lint clean.

Tests run by the reviewer on 600508c: mise run check:android exit 0 (114 tests); pixel6Api34DebugAndroidTest --rerun exit 0 (all managed tests pass). Pipeline for 600508c: 3/3 green.

Versions

pneumo-reviewer: round 1, REQUEST_CHANGES #

Findings #

Spec checks #

DoD checks #

pneumo-reviewer: round 2, APPROVE #

Findings #

Spec / DoD #