Add iOS Release-config UI test gate to TestFlight workflow (#253)#261
Merged
Conversation
|
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)
📝 WalkthroughWalkthroughAdds a Release-config pre-flight smoke test job to the iOS TestFlight CI, expands artifact uploads (DiagnosticReports/xcresult), tightens the e2e runner to require Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as macOS Runner
participant XcodeGen as xcodegen
participant Xcode as xcodebuild/Simulator
participant Artifacts as Artifact Storage
GH->>Runner: trigger pre-flight-tests job
Runner->>Runner: checkout, setup Node v22, install deps
Runner->>XcodeGen: install & run xcodegen (with retries)
Runner->>Xcode: run npm run e2e:ios:smoke (Release config)
Xcode->>Runner: test results + xcresult + DiagnosticReports
Runner->>Artifacts: upload artifacts (if: always(), ignore missing)
GH->>GH: build-and-upload job waits for pre-flight-tests completion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/QA_CHECKLIST.md`:
- Line 55: Update the checklist item that currently reads "Build a Release
archive locally and install via Xcode → Window → Devices and Simulators → drag
`.ipa`" to include the missing export step or point QA to the exact artifact;
for example, instruct users to "Build a Release archive, export an .ipa via
Xcode Organizer → Distribute App → Export → Ad Hoc/App Store Connect, then
install the exported .ipa via Xcode → Window → Devices and Simulators → drag
`.ipa`" so the QA step in QA_CHECKLIST.md is executable and unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c376381c-5702-4f28-84d6-c2d848569979
📒 Files selected for processing (4)
.github/workflows/e2e-ios.yml.github/workflows/ios-testflight.ymlios/QA_CHECKLIST.mdscripts/e2e/run-ios-tests.sh
auerbachb
added a commit
that referenced
this pull request
Apr 27, 2026
- 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>
This was referenced Apr 27, 2026
auerbachb
added a commit
that referenced
this pull request
Apr 27, 2026
* Fix AudioEngine NSException crash on Begin tap (#262) Build 9 still crashes on Begin despite the defensive currentDay clamp from #256. Symbolicated .ips from a real device (iPhone 17 Pro / iOS 26.3.1) shows the proximate cause is AVAudioEngine, not currentDay: +[NSException raise:format:] AVAudioEngineGraph::Initialize(NSError**) -[AVAudioEngine startAndReturnError:] [StillPoint warmUp closure] _dispatch_call_block_and_release (queue: com.stillpoint.audioengine) AVAudioEngine.start() raises an Objective-C NSException (not a Swift error) on iOS 26 when invoked on a "bare" engine — no source node connected to mainMixerNode. Swift try/catch cannot catch NSExceptions, so it propagates to std::terminate -> abort -> SIGABRT. The trigger is the previous "warm engine" pattern: AudioEngine.init() and AudioEngine.warmUp() both eagerly called engine.start() before any sound was scheduled. Fix: only start the engine inside playSynthesized() where attach + connect have already happened. - AudioEngine.init: drop the eager ensureEngineRunning() call. Keep configureAudioSession() and observers. - AudioEngine.warmUp: reactivate the audio session only (still useful after backgrounding); engine.start() is the playSynthesized path's responsibility. - resumeAfterInterruptionIfNeeded / handleWillEnterForeground: same — reconfigure session and clear flags; don't restart a bare engine. - ensureEngineRunning is now only called from playSynthesized, which attaches + connects a source node before starting. Bundles E2E coverage for issue #254's regression class: - CompletionView gets accessibility identifiers on the SESSION NOTE TextEditor, the Save note button, and the "Saved" indicator - testLaunchLoginCompleteSessionAndHistoryPersistence is extended to type a note, tap Save note, and assert the "Saved" indicator before returning home Together with #261's pre-flight gate, a future regression on either the Begin-button path or the end-of-session save will fail CI before TestFlight upload. Closes #262 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Remove dead configuration-change retry path (Bugbot Low) After the parent commit's fix, resumeAfterInterruptionIfNeeded() no longer calls engine.start(), so the pendingResumeAfterConfigurationChange flag and its AVAudioEngineConfigurationChange-driven retry handler are dead code. Removing all three (the field, the observer, and handleEngineConfigurationChange) keeps the state machine honest. The interruption-end path stays correct: handleInterruption(.ended) checks shouldResume && wasRunningBeforeInterruption directly and calls resumeAfterInterruptionIfNeeded() (which reactivates the session and returns; engine restart still happens lazily on next playSynthesized). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop dead wasRunningBeforeBackground state (Bugbot Low) After the prior dead-code removal, wasRunningBeforeBackground is only set + consumed locally in handleDidEnterBackground (the if-pause check) and reset to false in handleWillEnterForeground without ever being read. Replace with a direct engine.isRunning check in handleDidEnterBackground; drop the property and the foreground reset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 27, 2026
auerbachb
added a commit
that referenced
this pull request
Apr 27, 2026
) (#267) * Stabilize iOS UI tests by wiping all persisted state on resetStore (#266) 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> * Dedupe clearLocalSessionArtifacts -> clearPersistedSessionArtifacts (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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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>
- 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>
auerbachb
force-pushed
the
issue-253-ios-e2e-release-gate
branch
from
a42aab0 to
1c7fd57
Compare
|
CodeAnt AI is reviewing your PR. |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
|
||
| TEST_PLAN="${IOS_TEST_PLAN:-StillPointE2E}" | ||
| TEST_DESTINATION="${IOS_TEST_DESTINATION:-platform=iOS Simulator,name=iPhone 16,OS=latest}" | ||
| TEST_DESTINATION="${IOS_TEST_DESTINATION:-platform=iOS Simulator,name=iPhone 17,OS=latest}" |
There was a problem hiding this comment.
Suggestion: The default simulator destination is hardcoded to iPhone 17, which will make xcodebuild test fail immediately on machines where that exact simulator runtime/device is not installed. This turns the lane into an infra failure instead of a test signal. Use a more portable default (for example, detect an available iOS Simulator dynamically or require IOS_TEST_DESTINATION to be set in CI/local callers). [possible bug]
Severity Level: Major ⚠️
- ❌ E2E iOS PR checks can fail from missing simulator.
- ❌ TestFlight pre-flight UI gate can fail on env changes.
- ⚠️ Local `npm run e2e:ios:*` brittle to simulator availability.
- ⚠️ Test failures conflated with environment misconfiguration issues.Steps of Reproduction ✅
1. Trigger the iOS E2E workflow in CI via a pull request that touches iOS or E2E-related
files so `.github/workflows/e2e-ios.yml:1-23` runs job `ios-e2e` on `macos-26`.
2. In the `Run iOS ${{ matrix.lane }} lane` step (`.github/workflows/e2e-ios.yml:55-62`),
GitHub Actions executes `npm run e2e:ios:${{ matrix.lane }}`, which maps in
`package.json:23-24` to `bash scripts/e2e/run-ios-tests.sh <lane> 1` without setting
`IOS_TEST_DESTINATION`.
3. Inside `scripts/e2e/run-ios-tests.sh:50`, `TEST_DESTINATION` is computed as
`TEST_DESTINATION="${IOS_TEST_DESTINATION:-platform=iOS Simulator,name=iPhone
17,OS=latest}"`, so on these CI runs (and on any local `npm run e2e:ios:*` where
`IOS_TEST_DESTINATION` is unset) the hardcoded `iPhone 17` destination is passed to
`xcodebuild` via the `-destination "${TEST_DESTINATION}"` flag in `run_lane()`
(`scripts/e2e/run-ios-tests.sh:80-85,92-94`).
4. On any macOS runner or developer machine where the `platform=iOS Simulator,name=iPhone
17,OS=latest` simulator is not installed (a scenario explicitly acknowledged as variable
in `ios/E2E_RUNBOOK.md:15`, which tells callers to set `IOS_TEST_DESTINATION` to a
simulator "available in your local/CI environment"), `xcodebuild` will fail with a
destination-matching error before tests execute, causing the E2E iOS workflow and the
TestFlight pre-flight gate (`.github/workflows/ios-testflight.yml:54-62`, which also calls
`npm run e2e:ios:smoke` without `IOS_TEST_DESTINATION`) to report infrastructure-style
failures instead of meaningful UI test results.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/e2e/run-ios-tests.sh
**Line:** 50:50
**Comment:**
*Possible Bug: The default simulator destination is hardcoded to `iPhone 17`, which will make `xcodebuild test` fail immediately on machines where that exact simulator runtime/device is not installed. This turns the lane into an infra failure instead of a test signal. Use a more portable default (for example, detect an available iOS Simulator dynamically or require `IOS_TEST_DESTINATION` to be set in CI/local callers).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Owner
Author
There was a problem hiding this comment.
Fixed in 281d75a: replaced the hardcoded iPhone 17 default with a runtime fallback that picks the first installed iPhone from a preferred-order list (17 Pro → 17 → 17 Pro Max → 17e → 16e → 16 Pro → 16 → 15 Pro → 15 → Air) via xcrun simctl list devices available. IOS_TEST_DESTINATION env override is still honored. The selected destination is echoed into the CI log so future failures can be diagnosed faster.
|
CodeAnt AI finished reviewing your PR. |
#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>
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>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/APIClient.swift (1)
61-63: Scope E2E diagnostic logging to UI-test runs only.Line 62 logs
[E2E-DIAG]even when UI-test mode is off, which adds noise to normal app launches. Consider removing the non-UI-test branch or gating it to test-only mode.Proposed change
- } else { - print("[E2E-DIAG] APIClient.init uiTestMode=NO") - } + }🤖 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 61 - 63, The "[E2E-DIAG] APIClient.init uiTestMode=NO" diagnostic print is executed even when not running UI tests; update the APIClient initializer (the init that references uiTestMode) to either remove the else-branch print entirely or wrap it in a test-only guard so E2E logs only appear during UI-test runs—i.e., keep the print when uiTestMode is true and delete or conditionally compile the else branch (using a test-only flag or `#if` DEBUG && UITESTS) so normal app launches no longer emit the E2E-DIAG message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointApp/ViewModels/AppViewModel.swift`:
- Around line 73-79: Remove PII from the diagnostic print in the defer block of
checkAuth by stopping direct logging of currentUser.email and only emitting a
non-identifying marker when not in UI-test mode; instead, when running UI tests
(detect via a test flag like ProcessInfo.processInfo.arguments/Environment or an
existing isUITest flag), allow diagnostic output but mask the email (e.g.,
replace with "<redacted>" or a hashed/obfuscated token). Update the defer block
where isLoading, viewSlug(currentView), currentUser and startedAt are referenced
so the print uses a redactedCurrentUser variable (or omits currentUser) unless
the UI-test gate is true, and ensure you reference viewSlug(currentView),
currentUser, and startedAt when making this change.
---
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/APIClient.swift`:
- Around line 61-63: The "[E2E-DIAG] APIClient.init uiTestMode=NO" diagnostic
print is executed even when not running UI tests; update the APIClient
initializer (the init that references uiTestMode) to either remove the
else-branch print entirely or wrap it in a test-only guard so E2E logs only
appear during UI-test runs—i.e., keep the print when uiTestMode is true and
delete or conditionally compile the else branch (using a test-only flag or `#if`
DEBUG && UITESTS) so normal app launches no longer emit the E2E-DIAG message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5696f192-386f-4a6f-8af1-40c0fc566189
📒 Files selected for processing (6)
.github/workflows/e2e-ios.yml.github/workflows/ios-testflight.ymlios/QA_CHECKLIST.mdios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointShared/Sources/StillPointShared/APIClient.swiftscripts/e2e/run-ios-tests.sh
✅ Files skipped from review due to trivial changes (1)
- ios/QA_CHECKLIST.md
Comment on lines
+73
to
+79
| defer { | ||
| isLoading = false | ||
| // Diagnostic for issue #266 / PR #261 — log the post-checkAuth view | ||
| // state so we can correlate with APIClient.init's [E2E-DIAG] line | ||
| // when iOS UI tests fail on the auth-screen waitForExistence. | ||
| print("[E2E-DIAG] checkAuth.done currentView=\(viewSlug(currentView)) currentUser=\(currentUser?.email ?? "nil") elapsedMs=\(Int(Date().timeIntervalSince(startedAt) * 1_000))") | ||
| } |
There was a problem hiding this comment.
Remove PII from auth diagnostic logging and gate it to UI-test mode.
Line 78 logs currentUser email directly. That’s user identifier leakage in runtime logs outside CI test contexts.
Proposed fix
func checkAuth() async {
let startedAt = Date()
isLoading = true
defer {
isLoading = false
- // Diagnostic for issue `#266` / PR `#261` — log the post-checkAuth view
- // state so we can correlate with APIClient.init's [E2E-DIAG] line
- // when iOS UI tests fail on the auth-screen waitForExistence.
- print("[E2E-DIAG] checkAuth.done currentView=\(viewSlug(currentView)) currentUser=\(currentUser?.email ?? "nil") elapsedMs=\(Int(Date().timeIntervalSince(startedAt) * 1_000))")
+ if shouldEmitE2EDiagnostics {
+ print("[E2E-DIAG] checkAuth.done currentView=\(viewSlug(currentView)) currentUserPresent=\(currentUser != nil) elapsedMs=\(Int(Date().timeIntervalSince(startedAt) * 1_000))")
+ }
}
@@
}
+
+ private var shouldEmitE2EDiagnostics: Bool {
+ let raw = ProcessInfo.processInfo.environment["SP_UI_TEST_MODE"]?.lowercased()
+ return raw == "1" || raw == "true" || raw == "yes" || raw == "on"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| defer { | |
| isLoading = false | |
| // Diagnostic for issue #266 / PR #261 — log the post-checkAuth view | |
| // state so we can correlate with APIClient.init's [E2E-DIAG] line | |
| // when iOS UI tests fail on the auth-screen waitForExistence. | |
| print("[E2E-DIAG] checkAuth.done currentView=\(viewSlug(currentView)) currentUser=\(currentUser?.email ?? "nil") elapsedMs=\(Int(Date().timeIntervalSince(startedAt) * 1_000))") | |
| } | |
| defer { | |
| isLoading = false | |
| if shouldEmitE2EDiagnostics { | |
| print("[E2E-DIAG] checkAuth.done currentView=\(viewSlug(currentView)) currentUserPresent=\(currentUser != nil) elapsedMs=\(Int(Date().timeIntervalSince(startedAt) * 1_000))") | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/StillPointApp/ViewModels/AppViewModel.swift` around lines 73 - 79, Remove
PII from the diagnostic print in the defer block of checkAuth by stopping direct
logging of currentUser.email and only emitting a non-identifying marker when not
in UI-test mode; instead, when running UI tests (detect via a test flag like
ProcessInfo.processInfo.arguments/Environment or an existing isUITest flag),
allow diagnostic output but mask the email (e.g., replace with "<redacted>" or a
hashed/obfuscated token). Update the defer block where isLoading,
viewSlug(currentView), currentUser and startedAt are referenced so the print
uses a redactedCurrentUser variable (or omits currentUser) unless the UI-test
gate is true, and ensure you reference viewSlug(currentView), currentUser, and
startedAt when making this change.
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>
|
CodeAnt AI is running Incremental review |
Owner
Author
📌 Bypass-merging despite failing iOS e2e checksBypass-merging this PR because:
Open follow-ups:
Closing out the session here. |
codeant-ai
Bot
removed
the
size:L
This was referenced May 1, 2026
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR adds a Release-configuration UI smoke test job that must pass before the TestFlight upload job runs, ensuring crashes like the Begin button issue are caught by automated tests. It also treats missing UI test suites as failures so broken builds cannot progress to distribution. sequenceDiagram
participant Developer
participant CI as GitHub Actions
participant TestRunner as iOS test runner script
participant Simulator
participant TestFlight
Developer->>CI: Push change that triggers TestFlight workflow
CI->>CI: Run pre flight tests job
CI->>TestRunner: Run iOS smoke lane with Release configuration
TestRunner->>Simulator: Execute UI smoke test flow
Simulator-->>TestRunner: Return test result
alt Tests pass
TestRunner-->>CI: Report success
CI->>TestFlight: Build and upload iOS app
else Tests fail or suite missing
TestRunner-->>CI: Report failure
CI-->>Developer: Block upload and expose logs and artifacts
end
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR adds a pre-flight Release-configuration UI smoke test job to the iOS TestFlight workflow and hardens the iOS test runner so that missing or failing UI tests block the TestFlight upload. sequenceDiagram
participant Dev as Developer
participant CI as ios testflight workflow
participant UITest as UI smoke job
participant Runner as iOS test runner script
participant Build as Build and upload job
participant Store as TestFlight
Dev->>CI: Trigger ios testflight workflow
CI->>UITest: Start pre flight tests job
UITest->>Runner: Run smoke lane in Release config
Runner->>Runner: Validate suite and run UI tests on simulator
Runner-->>UITest: Test result
alt UI tests pass
UITest-->>CI: Job success
CI->>Build: Run build and upload job
Build->>Store: Upload iOS build
else UI tests fail or suite missing
UITest-->>CI: Job failure
CI--XBuild: Skip build and upload
end
Generated by CodeAnt AI |
Comment on lines
+61
to
63
| } else { | ||
| print("[E2E-DIAG] APIClient.init uiTestMode=NO") | ||
| } |
There was a problem hiding this comment.
Suggestion: This diagnostic print runs whenever UI-test mode is not enabled, which includes normal production app launches. That causes unconditional runtime logging in non-test builds and contradicts the stated intent that diagnostics are test-only. Gate this branch behind a debug/test condition (or remove it) so production builds do not emit E2E diagnostic logs. [logic error]
Severity Level: Minor 🧹
- ⚠️ Production launches emit internal "[E2E-DIAG]" diagnostic log lines.
- ⚠️ Test-only diagnostics pollute normal device and crash logs.
- ⚠️ Minor startup overhead from unconditional console printing.Steps of Reproduction ✅
1. Launch the iOS app built from this PR in a normal (non-UI-test) configuration so that
the main entry point `StillPointApp` runs (ios/StillPointApp/StillPointApp.swift:5-13),
which creates a `WindowGroup` showing `RootView()`.
2. Observe that `RootView` (ios/StillPointApp/Views/RootView.swift:4-7) holds `@State
private var appVM = AppViewModel()` and attaches a `.task { await appVM.checkAuth() }`
modifier (RootView.swift:12-13), so `AppViewModel.checkAuth()` is invoked on initial view
appearance on every cold start.
3. In `checkAuth()` (ios/StillPointApp/ViewModels/AppViewModel.swift:70-87), the method
calls `APIClient.shared.me()` (AppViewModel.swift:85), which lazily initializes the
singleton `APIClient.shared` defined in `APIClient`
(ios/StillPointShared/Sources/StillPointShared/APIClient.swift:5-8, 16-23).
4. During `APIClient.init` (APIClient.swift:16-63), `UITestConfig.fromProcessInfo()` is
evaluated (line 29) and typically returns `nil` for normal production/dev launches as
described by the comment at lines 55-59 ("UITestConfig is nil in production"). That causes
the `else` branch at lines 61-63 to execute, which runs `print("[E2E-DIAG] APIClient.init
uiTestMode=NO")` unconditionally for every non-UI-test startup, emitting E2E diagnostic
logging in non-test builds.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** ios/StillPointShared/Sources/StillPointShared/APIClient.swift
**Line:** 61:63
**Comment:**
*Logic Error: This diagnostic print runs whenever UI-test mode is not enabled, which includes normal production app launches. That causes unconditional runtime logging in non-test builds and contradicts the stated intent that diagnostics are test-only. Gate this branch behind a debug/test condition (or remove it) so production builds do not emit E2E diagnostic logs.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+64
to
+86
| local preferred=( | ||
| "iPhone 17 Pro" | ||
| "iPhone 17" | ||
| "iPhone 17 Pro Max" | ||
| "iPhone 17e" | ||
| "iPhone 16e" | ||
| "iPhone 16 Pro" | ||
| "iPhone 16" | ||
| "iPhone 15 Pro" | ||
| "iPhone 15" | ||
| "iPhone Air" | ||
| ) | ||
| local name | ||
| for name in "${preferred[@]}"; do | ||
| if grep -F " ${name} (" <<<"${simctl_output}" >/dev/null 2>&1; then | ||
| echo "platform=iOS Simulator,name=${name},OS=latest" | ||
| return | ||
| fi | ||
| done | ||
|
|
||
| # Last-resort fallback: keep the prior hardcoded default so we fail with a | ||
| # clear xcodebuild error instead of an empty destination. | ||
| echo "platform=iOS Simulator,name=iPhone 17,OS=latest" |
There was a problem hiding this comment.
Suggestion: Simulator auto-selection only checks a fixed shortlist and then falls back to a hardcoded iPhone 17. On hosts where none of these names exist (but other iPhone simulators do), the script will fail unnecessarily instead of using an available simulator. Add a true generic fallback that parses simctl output for any available iPhone device before defaulting to a hardcoded model. [possible bug]
Severity Level: Major ⚠️
- ⚠️ iOS smoke lane can fail on future runner images.
- ⚠️ TestFlight pre-flight gate may block uploads unnecessarily.
- ⚠️ Local `npm run e2e:ios:smoke` can fail on some Macs.Steps of Reproduction ✅
1. Note that `resolve_test_destination()` in `scripts/e2e/run-ios-tests.sh:50-87` calls
`xcrun simctl list devices available` and stores the output in `simctl_output` at lines
62-63.
2. The function then iterates a fixed `preferred` list of device names (`iPhone 17 Pro`,
`iPhone 17`, `iPhone 17 Pro Max`, `iPhone 17e`, `iPhone 16e`, `iPhone 16 Pro`, `iPhone
16`, `iPhone 15 Pro`, `iPhone 15`, `iPhone Air`) defined at
`scripts/e2e/run-ios-tests.sh:64-75`, picking the first name whose exact line appears in
`simctl_output`.
3. On a host whose available simulators (from `xcrun simctl list devices available`)
include only other iPhone models (for example, `iPhone 14` or a future `iPhone 18`) and
none of the hardcoded `preferred` names, the loop at lines 77-81 finds no match and falls
through.
4. In that case, `resolve_test_destination()` returns the hardcoded fallback `platform=iOS
Simulator,name=iPhone 17,OS=latest` at `scripts/e2e/run-ios-tests.sh:84-86`, even though
an `iPhone` simulator is available but named differently.
5. When `run_ios_tests.sh` is invoked via `npm run e2e:ios:smoke` (defined in
`package.json:23` and used by `.github/workflows/ios-testflight.yml:15-23`),
`TEST_DESTINATION` (lines 89-90) is set to the unsupported `iPhone 17` destination,
causing the `xcodebuild` invocation at `scripts/e2e/run-ios-tests.sh:120-127` to fail with
"No devices are available that match the requested destination", and the
`pre-flight-tests` job fails even though a valid iPhone simulator exists.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/e2e/run-ios-tests.sh
**Line:** 64:86
**Comment:**
*Possible Bug: Simulator auto-selection only checks a fixed shortlist and then falls back to a hardcoded `iPhone 17`. On hosts where none of these names exist (but other iPhone simulators do), the script will fail unnecessarily instead of using an available simulator. Add a true generic fallback that parses `simctl` output for any available iPhone device before defaulting to a hardcoded model.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:L
Sequence DiagramThis diagram shows how the TestFlight workflow now runs a Release-config iOS UI smoke test before building, and blocks the TestFlight upload if the UI test suite is missing or fails. sequenceDiagram
participant Workflow as TestFlight workflow
participant PreFlight as Pre-flight tests job
participant Runner as iOS E2E runner script
participant Simulator as iOS simulator tests
participant Build as Build and upload job
Workflow->>PreFlight: Run Release UI smoke gate
PreFlight->>Runner: Invoke smoke lane with Release configuration
Runner->>Runner: Validate UI test bundle path
Runner->>Runner: Resolve available simulator destination
Runner->>Simulator: Run xcode tests for smoke UI path
alt UI tests pass
Simulator-->>PreFlight: Report success
PreFlight-->>Workflow: Job succeeded
Workflow->>Build: Start build and upload
else UI tests fail or suite missing
Simulator-->>PreFlight: Report failure
PreFlight-->>Workflow: Job failed
Workflow-->>Build: Block TestFlight upload
end
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
|
CodeAnt AI is running the review. |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR adds a pre-flight job that runs iOS UI smoke tests in Release configuration before TestFlight uploads and hardens the iOS E2E runner so missing or failing tests block the release workflow. sequenceDiagram
participant Developer
participant CI
participant UITestRunner
participant BuildJob
participant TestFlight
Developer->>CI: Push iOS release tag
CI->>CI: Start ios-testflight workflow
CI->>UITestRunner: Run Release-config smoke lane
UITestRunner->>UITestRunner: Execute UI tests on simulator
UITestRunner-->>CI: Report test result
alt Tests pass
CI->>BuildJob: Start build and upload job
BuildJob->>TestFlight: Upload signed iOS build
TestFlight-->>Developer: Build available for testing
else Tests fail or suite missing
CI-->>Developer: Fail workflow and block TestFlight upload
end
Generated by CodeAnt AI |
|
CodeAnt AI finished running the review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
Closes the E2E gap that let the build 8 Begin-button crash (#250) reach the App Store despite a UI test existing that should have caught it.
Root cause for the silent miss (CI logs from PR #260):
scripts/e2e/run-ios-tests.shpointed atios/StillPointUITests(real path isios/StillPointAppUITests), referenced a nonexistentStillPointE2Etest plan, and on missing-directory setFINAL_STATUS="skipped" + exit 0. Theios-e2e-smoke/ios-e2e-criticalCI checks have been "passing" while running zero tests.Changes
1. Fix the runner (scripts/e2e/run-ios-tests.sh)
IOS_UI_TESTS_DIRdefault →ios/StillPointAppUITests-testPlan StillPointE2E(no.xctestplanexists)smoke→StillPointAppUITests/StillPointAppUITests/testLaunchLoginCompleteSessionAndHistoryPersistence(Begin → Session → Complete → History)critical→StillPointAppUITests/StillPointAppUITests(whole class)IOS_TEST_CONFIGURATIONknob so callers can pin Release configiPhone 16→iPhone 17(iPhone 16 doesn't exist on macos-26 runners)2. Release-config pre-flight gate (.github/workflows/ios-testflight.yml)
pre-flight-testsjob runs the smoke lane withIOS_TEST_CONFIGURATION=Releaseon simulatorbuild-and-uploadnowneeds: pre-flight-tests— a failing UI test blocks the TestFlight upload~/Library/Logs/DiagnosticReports/**+ xcresult bundle on every run3. Defense in depth on PR-time runs (.github/workflows/e2e-ios.yml)
4. Manual pre-tag smoke (ios/QA_CHECKLIST.md)
ios-v*tagAfter this PR was opened, fixing the silent-skip bug immediately surfaced 3 pre-existing UI test failures:
testHistoryAndSettingsNavigationSmoketruetestKeyboardOverlapKeepsSubmitReachablefalseXCTAssertTrue(authRoot.waitForExistence(timeout: launchTimeout))testLaunchLoginCompleteSessionAndHistoryPersistencefalsetestLaunchOfflineShowsUserVisibleMessagetrue, forceLaunchOffline: trueThe pattern: every test expecting the auth screen fails. The single test expecting home (with
seedAuthenticated: true) passes. The fail mode is identical:Auth screen did not appearafterwaitForExistencetimes out.Things tried that did NOT fix it
resetStore: true. The originaldefaults.removeObject(forKey: uiTestStoreDefaultsKey)only wiped the in-memory UI test store. Now wipes everything. Did not fix.launchTimeoutfrom 15s → 30s. Failure still happens within the 30s window — the test's 55s total runtime indicates the auth-screen wait timed out at 30s, then teardown took ~25s. Did not fix.defaults.synchronize()after the reset-store wipe in case cfprefsd cross-process caching was masking the wipe. Pushed in266394b.What I'm trying next (latest commit on this branch)
Added
[E2E-DIAG]print()calls in two places:APIClient.init— logs parsedUITestConfig+ finaluiTestStore.isAuthenticatedAppViewModel.checkAuth— logs finalcurrentView+ elapsed msGoal: see in the next CI run's xcresult logs whether the env vars are being honored, what
isAuthenticatedresolves to, and whatcurrentViewends up as. That'll tell us:[E2E-DIAG] APIClient.init uiTestMode=NOappears → env vars aren't being passed to the launched app process (test fixture issue)[E2E-DIAG] APIClient.init ... finalIsAuthenticated=truedespiteseedAuth=false, resetStore=true→ the reset-store path isn't producing a fresh false-auth store (likely UserDefaults persistence / cfprefsd issue)[E2E-DIAG] checkAuth.done currentView=home currentUser=ios.fixture@stillpoint.testdespite seedAuth false →me()is somehow returning a user (most likely cause:uiTestMereading stale store)[E2E-DIAG] checkAuth.done currentView=authbut the test still fails → SwiftUI rendering / accessibility identifier issue under iOS 26 simulatorWhat I've ruled out
XCUIApplication.launch()is a new app processCoordination
This PR depends on #267 being merged (and is rebased on top). User-facing P0 (Begin crash) is already shipped via build 10 on TestFlight — verified working on iPhone 17 Pro / iOS 26.3.1.
Recommendation if diagnostics don't crack it: park PR #261 with the gate infrastructure correct, file a follow-up for the underlying simulator-test issue, and ship future iOS releases via the manual pre-tag smoke checklist (already added to QA_CHECKLIST.md by this PR) until the simulator gate stabilizes.
Test plan
ios-e2e-smoke/ios-e2e-criticalCI logs show real test execution (not "iOS E2E suite not present")[E2E-DIAG]lines appear in xcresult bundle for the next CI run on this branchRelated
Closes #253
Summary by CodeRabbit
Release Notes
CodeAnt-AI Description
Add a Release-config UI smoke gate before TestFlight upload
What Changed
Impact
✅ Fewer TestFlight uploads with untested iOS builds✅ Clearer iOS UI test failures in CI✅ Easier recovery after release-blocking app crashes🔄 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.