Repository navigation
Conversation
a6584c6 to
851a882
Compare
|
@codex review |
|
@MelvinBot review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 851a882984
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
🤖 ReviewThe JSON is valid and the domain choice is right, but the PR description is wrong on two counts and the claim ships a small regression before anything can use it. 1. This PR alone activates the claim on production iOS — the Mobile-Expensify PR doesn't gate it. The description says the matching Associated Domains entitlement "lives in Mobile-Expensify and is a separate PR." That's only true for 2. Nothing on iOS consumes QA auth is hard-off on native: So a real tap on There's a direct precedent: 3. The Android claim in the description is incorrect.
4. Question: will the callback arrive as a Universal Link at all? The flow ends with Cloudflare issuing a server-side 302 back to What checks out
Why I didn't run an app testThis is a static claim file with no runtime behaviour in the web or Android app, and the iOS effect can only be observed on a physical device against a deployed AASA. Nothing here is verifiable in the automated test environment, so I reviewed statically instead. Reply with view run · no recording available |
|
@MelvinBot re-review please, all four raised points addressed |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
🤖 Re-reviewPoints 1, 3 and 4 are addressed. Point 2 is half-fixed: The gap: The But
Signed in, this is fine (it bails at 607 and the filter handles it). Signed out, the user gets dumped on NotFound right after signing in. This is the same shape as the Plaid precedent I flagged last time: that path needs its bail-out in two places, Two minor notes
What checks out
Why there's no app testI attempted a web smoke test to confirm the shared It also wouldn't have observed anything: Reply with |
|
@MelvinBot yeah, valid, added the same guard next to the Plaid one, and reworded the filter comment to say native only. Please re-review |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
🤖 Re-reviewBoth changes are right — the gap is closed and I have nothing blocking left. The guard at The only other URL consumer I found is One optional leftover, unchanged from last round and purely cosmetic: a filtered callback URL still counts as a "specific deep link" at CI is green — 27 checks passing, perf tests still running, and 🤖 Web test steps verification results: ✅ passAll three regression checks pass on dev NewDot web: Home/inbox render correctly, the Concierge chat/report loads, and account menu navigation to Profile works. No console or network errors observed.
This exercises the shared code the diff touches — Reply with |
/oauth/callback as an iOS universal link/oauth/native-callback as an iOS universal link
|
I will take a look this pr soon |
dylanexpensify
left a comment
There was a problem hiding this comment.
No product review required
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
huult
left a comment
There was a problem hiding this comment.
The code changes LGTM. We can't verify this until it is deployed, so I'll verify it once it's deployed.
|
🚧 iwiznia has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/iwiznia in version: 9.4.74-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.74-2 🚀
|
ScreenRecording_09-11-2026.20-48-24_1.MP4
@iwiznia @JakubKorytko This PR has been deployed to production, and I verified that it’s working as expected. |




Explanation of Change
Claims
/oauth/native-callbackas an iOS Universal Link so the Cloudflare Access OAuth callback of the nativesign-in flow (
ASWebAuthenticationSessionwith an https callback, iOS 17.4+) returns to the app. Onecomponentsentry added to.well-known/apple-app-site-association, next to the existing Plaid OAuth entry.appIDsare untouched.The path is native-only on purpose. Web keeps
/oauth/callback, which no app claims, so a sign-in started inSafari stays in Safari where its PKCE state lives.
Because the standalone NewDot entitlements in this repo already list
applinks:new.expensify.comandapplinks:staging.new.expensify.com, this claim is live on production iOS as soon as the file deploys, withoutan app release. Since no screen exists at the path,
linkingConfig.filterdrops it for both cold start andwarm
urlevents so a tap never lands on NotFound (isNativeOAuthCallbackURL, unit tested).Android is not covered here:
assetlinks.jsonhandle_all_urlsis only the verification relation, theintent-filter in Mobile-Expensify decides what opens the app and is a separate PR, as is the HybridApp
entitlement.
Fixed Issues
$ #91419
PROPOSAL: N/A
Tests
This is a static claim file with no runtime behaviour in the app, so there is nothing to click. It is only
verifiable once deployed, and only together with the Mobile-Expensify entitlement PR.
curl -s https://new.expensify.com/apple-app-site-association | jq '.applinks.details[0].components[] | select(."/" == "/oauth/native-callback")'Content-Typeisapplication/jsonhttps://new.expensify.com/settingsand tap it (the Safari address bar does not trigger Universal Links)no existing claim regressed
Offline tests
N/A
QA Steps
https://staging.new.expensify.com/settingshandling is unaffected
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari