Stabilize iOS UI tests by wiping all persisted state on resetStore (#266)#267
Conversation
) Background: PR #261 unmasked the silent-skip bug in scripts/e2e/run-ios-tests.sh (which had the wrong test directory path and exited 0 on missing tests). With that fixed, three pre-existing UI tests started failing in CI: - testKeyboardOverlapKeepsSubmitReachable (seedAuthenticated: false) - testLaunchLoginCompleteSessionAndHistoryPersistence (seedAuthenticated: false) - testLaunchOfflineShowsUserVisibleMessage (forceLaunchOffline: true) The pattern: every failure is on the auth-screen waitForExistence assertion at the start of the test, while testHistoryAndSettingsNavigationSmoke (seedAuthenticated: true) passed. Two contributing causes: 1. resetStore: true only wiped the in-memory uiTestStore, not Keychain (AuthTokenStore), HTTP cookies, URL credentials, or AudioEngine sound prefs. So state from a prior seedAuthenticated:true test could bleed into subsequent seedAuthenticated:false tests. 2. launchTimeout = 15s was tight for first-launch on macos-26 runners with iPhone 17 simulators booting cold. Fix: belt-and-suspenders. - APIClient.init: when resetStore is true, also call a new static helper that mirrors clearLocalSessionArtifacts() — clears Keychain, cookies, URL credentials. Plus AudioEngine.resetPersistedPrefs() for sound prefs. - AudioEngine: new resetPersistedPrefs() static that clears the stillpoint_sound_prefs UserDefaults entry. - StillPointAppUITests: launchTimeout 15 -> 30 with explanatory comment. Together, these address the three currently-failing tests AND make the reset-store contract honest (when a test asks for a clean slate, it gets one). Closes #266 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes extend the UI-test reset path to clear persisted state including audio preferences, session cookies, authentication tokens, and credentials, while increasing the app launch timeout from 15 to 30 seconds to reduce intermittent failures on cold simulator boots. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7cc7b49. Configure here.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/APIClient.swift (1)
54-70: Deduplicate the session-artifact wipe.
clearPersistedSessionArtifacts(session:)now mirrorsclearLocalSessionArtifacts()almost exactly. Keeping both implementations in sync is drift-prone the next time another persisted artifact gets added.♻️ Minimal consolidation
private func clearLocalSessionArtifacts() { - _ = AuthTokenStore.clear() - - let cookieStorage = session.configuration.httpCookieStorage ?? .shared - for cookie in cookieStorage.cookies ?? [] { - cookieStorage.deleteCookie(cookie) - } - - let credentialStorage = URLCredentialStorage.shared - for (protectionSpace, credentialsByUser) in credentialStorage.allCredentials { - for credential in credentialsByUser.values { - credentialStorage.remove(credential, for: protectionSpace) - } - } + Self.clearPersistedSessionArtifacts(session: session) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift` around lines 54 - 70, The two nearly identical implementations clearPersistedSessionArtifacts(session:) and clearLocalSessionArtifacts() should be consolidated to avoid drift: extract the common wipe logic (AuthTokenStore.clear(), deleting cookies from a given HTTPCookieStorage, and removing credentials from URLCredentialStorage.allCredentials) into a single private helper (e.g., private static func wipePersistedSessionArtifacts(cookieStorage: HTTPCookieStorage?)) and then have clearPersistedSessionArtifacts(session:) call that helper with session.configuration.httpCookieStorage and have clearLocalSessionArtifacts() call the same helper with .shared; ensure the helper also performs the credential removal using URLCredentialStorage.shared so both call sites reuse identical code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift`:
- Around line 54-70: The two nearly identical implementations
clearPersistedSessionArtifacts(session:) and clearLocalSessionArtifacts() should
be consolidated to avoid drift: extract the common wipe logic
(AuthTokenStore.clear(), deleting cookies from a given HTTPCookieStorage, and
removing credentials from URLCredentialStorage.allCredentials) into a single
private helper (e.g., private static func
wipePersistedSessionArtifacts(cookieStorage: HTTPCookieStorage?)) and then have
clearPersistedSessionArtifacts(session:) call that helper with
session.configuration.httpCookieStorage and have clearLocalSessionArtifacts()
call the same helper with .shared; ensure the helper also performs the
credential removal using URLCredentialStorage.shared so both call sites reuse
identical code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c5fa543-8bc5-4a30-b490-0955ea1d5329
📒 Files selected for processing (3)
ios/StillPointAppUITests/StillPointAppUITests.swiftios/StillPointShared/Sources/StillPointShared/APIClient.swiftios/StillPointShared/Sources/StillPointShared/AudioEngine.swift
|
CodeAnt AI finished reviewing your PR. |
…Bugbot/CR) Both reviewers flagged that the new static helper was a line-for-line duplicate of the existing instance method. Make the instance method delegate to the static so cleanup logic has a single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#266) After PR #267's reset-store wipe + 30s launchTimeout bump, three iOS UI tests still fail on the same auth-screen waitForExistence assertion. The fix didn't move the needle on the actual symptom, so before pushing more fixes blind, instrument the suspect surfaces: - APIClient.init: print parsed UITestConfig + final isAuthenticated. This tells us whether SP_UI_TEST_MODE is being honored and whether the reset-store path actually produced a fresh isAuthenticated=false store. - AppViewModel.checkAuth: print final currentView slug + currentUser email + elapsedMs, so we can correlate with APIClient.init and see if the app is landing on .auth/.home/something else. - Defensive defaults.synchronize() after the reset wipe in case there's a cross-process cfprefsd timing issue specific to macos-26 + iOS 26 simulator. The print() lines are guarded by uiTestConfig != nil so they're noise-free in production builds. Refs #253, #266, PR #261 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add iOS Release-config UI test gate to TestFlight workflow (#253) The iOS Begin-button crash (issue #250) shipped in App Store build 8 despite having a UI test that taps Begin and asserts SessionView mounts. Root cause for the gap: scripts/e2e/run-ios-tests.sh pointed at ios/StillPointUITests (the actual directory is ios/StillPointAppUITests), and on missing-directory it set FINAL_STATUS="skipped" + exit 0 — so the ios-e2e-smoke / ios-e2e-critical CI checks have been "passing" without running anything for an extended period. Logs from PR #260 confirm: "iOS E2E suite not present (ios/StillPointUITests); skipping critical lane." This PR closes the gap on three layers: 1. Fix the runner script: - IOS_UI_TESTS_DIR default -> ios/StillPointAppUITests - Drop bogus -testPlan StillPointE2E (no .xctestplan exists) - Lane mapping points at the real classes/methods: smoke -> StillPointAppUITests/StillPointAppUITests/testLaunchLoginCompleteSessionAndHistoryPersistence critical -> StillPointAppUITests/StillPointAppUITests - Hard-fail (exit 1) on missing test directory, never silently skip - Add IOS_TEST_CONFIGURATION knob so callers can pin Release config 2. Add a Release-config pre-flight gate to ios-testflight.yml: - New pre-flight-tests job runs the smoke lane with IOS_TEST_CONFIGURATION=Release on simulator - build-and-upload now `needs: pre-flight-tests` — a failing UI test blocks the TestFlight upload outright - Uploads simulator DiagnosticReports + xcresult bundle on every run 3. Defense in depth on PR-time runs: - e2e-ios.yml uploads simulator DiagnosticReports as artifact too, so any future silent crash leaves a recoverable trace - ios/QA_CHECKLIST.md gains a Pre-tag manual smoke section for the real-device pass that the simulator gate cannot fully replace After this lands, intentionally re-introducing the build 8 bug on a throwaway branch should fail the pre-flight gate and block a hypothetical TestFlight tag push, satisfying the issue's primary acceptance criterion. Closes #253 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix iOS e2e simulator destination + CR Minor (archive vs ipa) - run-ios-tests.sh: default simulator iPhone 16 -> iPhone 17. macos-26 runners no longer have an "iPhone 16" simulator (only "iPhone 16e"), so xcodebuild was failing immediately with "Unable to find a device matching the provided destination specifier". This was the actual cause of the iOS e2e failures on this branch — once the runner script stopped silently skipping (per the parent fix in this PR), it surfaced the simulator mismatch as a real failure. iPhone 17 is the runner's current default phone simulator. - QA_CHECKLIST.md: clarify the install step. An xcarchive is not directly installable; reword to either (a) install the TestFlight build via the TestFlight app (preferred) or (b) export an Ad Hoc .ipa and drag it onto the device. Addresses CR Minor on PR #261. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add [E2E-DIAG] launch-time diagnostics + UserDefaults synchronize (#253/#266) After PR #267's reset-store wipe + 30s launchTimeout bump, three iOS UI tests still fail on the same auth-screen waitForExistence assertion. The fix didn't move the needle on the actual symptom, so before pushing more fixes blind, instrument the suspect surfaces: - APIClient.init: print parsed UITestConfig + final isAuthenticated. This tells us whether SP_UI_TEST_MODE is being honored and whether the reset-store path actually produced a fresh isAuthenticated=false store. - AppViewModel.checkAuth: print final currentView slug + currentUser email + elapsedMs, so we can correlate with APIClient.init and see if the app is landing on .auth/.home/something else. - Defensive defaults.synchronize() after the reset wipe in case there's a cross-process cfprefsd timing issue specific to macos-26 + iOS 26 simulator. The print() lines are guarded by uiTestConfig != nil so they're noise-free in production builds. Refs #253, #266, PR #261 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Pick first available iPhone simulator instead of hardcoding (CodeAnt) CodeAnt flagged: hardcoding `iPhone 17` as the default has the same infra-failure mode that bit us when the prior default `iPhone 16` was removed from the macos-26 runner image. Future runner refreshes will break this lane the same way. Replace the hardcoded default with a runtime fallback chain. The new resolve_test_destination function: - Honors IOS_TEST_DESTINATION env var if explicitly set (unchanged) - Otherwise greps `xcrun simctl list devices available` for the first installed iPhone from a preferred-order list (17 Pro, 17, 17 Pro Max, 17e, 16e, 16 Pro, 16, 15 Pro, 15, Air) - Last-resort fallback to the prior hardcoded default so xcodebuild's own error message surfaces if the host has no iPhone simulators at all Echoes the resolved destination into the CI log so future failures show exactly which simulator was selected. Refs PR #261, addresses CodeAnt Suggestion finding from 2026-04-27. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Gate [E2E-DIAG] checkAuth print on SP_UI_TEST_MODE (CodeAnt PII) CodeAnt flagged: the diagnostic print included currentUser.email, which would leak PII into production logs. Gate the entire print on SP_UI_TEST_MODE=1 so the diag only emits during UI-test runs (the only context where it's useful) and is silent in production. The fixture sets SP_UI_TEST_MODE=1 via launchEnvironment, so test runs keep the diagnostic for debugging while production builds emit nothing. Refs PR #261 / CodeAnt finding 2026-04-27. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is running the review. |
Sequence DiagramThis PR ensures that when UI tests launch the app with resetStore enabled, all persisted session state (auth tokens, cookies, credentials, and sound preferences) is cleared so each test starts from a clean slate, and also relaxes the app launch timeout for CI stability. sequenceDiagram
participant UITest
participant App
participant APIClient
participant Storage
UITest->>App: Launch with resetStore true
App->>APIClient: Initialize client
APIClient->>APIClient: Parse UI test configuration
APIClient->>Storage: Clear UI test store and session artifacts
Storage-->>APIClient: Persisted state removed
APIClient-->>App: Client ready with clean state
App-->>UITest: Present initial UI for test
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
Sequence DiagramThis PR ensures that when iOS UI tests launch the app with resetStore enabled, all persisted session artifacts and audio preferences are wiped so each test starts from a clean slate. sequenceDiagram
participant UITest
participant App
participant APIClient
participant PersistedState
participant Audio
UITest->>App: Launch app with resetStore true and seedAuthenticated flag
App->>APIClient: Initialize API client
APIClient->>PersistedState: Clear UI test store, auth tokens, cookies, URL credentials
APIClient->>Audio: Reset persisted sound preferences
APIClient->>PersistedState: Load or create UI test store based on seedAuthenticated
App-->>UITest: App ready with clean session state for test
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |

User description
Summary
Closes the test-state pollution that PR #261 surfaced after fixing the runner script's silent-skip bug. Three iOS UI tests began failing on CI:
All failed at the auth-screen `waitForExistence` assertion at the start of each test, while `testHistoryAndSettingsNavigationSmoke` (`seedAuthenticated: true`) passed.
Root cause (two contributing factors)
1. `resetStore: true` was only wiping the in-memory store
`APIClient.init()` lines 31-44 called `defaults.removeObject(forKey: uiTestStoreDefaultsKey)` but never touched:
So state from a prior `seedAuthenticated: true` test bled into a subsequent `seedAuthenticated: false` test.
2. `launchTimeout = 15s` was tight for cold-launch on macos-26 runners
The first `XCUIApplication.launch()` on a fresh iPhone 17 simulator can take 15s+ just to attach the test bundle and render the first frame.
Fix (belt-and-suspenders)
`APIClient.swift`
`init()` now calls a new static `clearPersistedSessionArtifacts(session:)` when `resetStore: true` — mirrors `clearLocalSessionArtifacts()` but is a static so it can run during init before the actor is fully initialized. Wipes Keychain, cookies, URL credentials. Also calls `AudioEngine.resetPersistedPrefs()`.
`AudioEngine.swift`
New `public static func resetPersistedPrefs()` — wipes the `stillpoint_sound_prefs` UserDefaults entry. Used by the UI-test reset path; also useful for any future "factory reset" surface.
`StillPointAppUITests.swift`
`launchTimeout` 15s → 30s with explanatory comment.
Test plan
Coordination with PR #261
This PR is the prerequisite for PR #261 to merge. Order:
Severity
P1 — blocks the E2E release gate from #253. Without that gate, future releases can re-introduce regressions like #250 / #262 with no automated catch.
Closes #266
🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
CodeAnt-AI Description
Stabilize iOS UI tests by fully clearing test state and allowing more time for first launch
What Changed
Impact
✅ Fewer flaky iOS UI tests✅ Cleaner sign-in resets between test runs✅ Fewer first-launch timeouts on CI🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.