Skip to content

[CP Staging] fix deeplink issue - #101528

Merged
lakchote merged 2 commits into
Expensify:mainfrom
nkdengineer:fix/101525
Sep 18, 2026
Merged

lakchote merged 2 commits into
Expensify:mainfrom
nkdengineer:fix/101525

Conversation

@nkdengineer

@nkdengineer nkdengineer commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

fix deeplink issue

Fixed Issues

$ #101525
#101551
PROPOSAL:

Tests

Precondition:

  • Copy the DM conversation URL and paste it to slack
  • Log out from New dot app

Steps:

  1. Tap the conversation link from Slack
  2. In the log in screen enter the same user
  3. Verify that the user is navigated to the DM chat
  • Verify that no errors appear in the JS console

Offline tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Screen.Recording.2026-09-18.at.14.46.45.mov
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

@lakchote lakchote changed the title fix deeplink issue [CP Staging] fix deeplink issue Sep 18, 2026
@codecov

codecov Bot commented Sep 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/libs/actions/Link.ts 55.23% <0.00%> (ø)
... and 11 files with indirect coverage changes

@nkdengineer
nkdengineer marked this pull request as ready for review September 18, 2026 08:02
@nkdengineer
nkdengineer requested review from a team as code owners September 18, 2026 08:02
@melvin-bot

melvin-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

@abdulrahuman5196 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot removed request for a team September 18, 2026 08:03
@lakchote
lakchote requested review from aimane-chnaif and removed request for abdulrahuman5196 September 18, 2026 11:07
@aimane-chnaif

aimane-chnaif commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
101525.mov
101551.mov
Android: mWeb Chrome
iOS: HybridApp
iOS: mWeb Safari
MacOS: Chrome / Safari
101525.mov
101551.mov

Comment thread src/libs/actions/Link.ts
@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Review

Blocking: this flip re-opens [$250] Public room-anon user first time tapping on sign in button or a thread open public room, the bug the guard was added for one day ago in Fix: anonymous user first time tapping on sign in button or a thread open public room. The guard was the only behavioral change in that PR, and this inverts it — so on the #96346 path we're back to pre-#99582 behavior. As a [CP Staging] that trades two regressions for a third on the same build.

To be clear: the diagnosis is right — the old !isActiveRoute condition does drop every signed-out deeplink after sign-in, which is exactly iOS - Logged out user is not redirected to the correct conversation when navigate via link. But neither polarity is correct. isActiveRoute answers "is the deeplink focused right now", and both bugs need "did the user ever reach the deeplinked screen".

Suggested fix — keep your new skip, then add the discriminator:

const deeplinkRoute = route as Route;
// Already focused — nothing to do.
if (deeplinkRoute && Navigation.isActiveRoute(deeplinkRoute)) {
    return;
}
// The deeplinked report is still mounted under what the user just opened (sign-in RHP or a thread), so they
// reached it and moved on — re-navigating would yank them back (#96346). If it isn't in the tree at all they
// never got there (they were on the sign-in screen), so the deeplink must still be honored (#101525/#101551).
if (reportID && Navigation.getReportRouteByID(reportID)) {
    return;
}

getReportRouteByID already walks the whole navigation tree, and is already exported. One caveat to check on device: it only walks state.routes, not RN7's preloadedRoutes, so confirm LHN preloading doesn't produce a false positive.

Why #96346 comes back — evidence
  1. isActiveRoute compares the focused route only. Navigation.ts:303-309getActiveRouteWithoutParams()getPathFromState.ts:181-184, which builds the path from findFocusedRouteWithOnyxTabGuard(state). The central pane underneath is not part of the path.

  2. Tapping anonymous "Sign in" changes the active route. Session/index.ts:389-395 navigates to ROUTES.SIGN_IN_MODAL, an RHP screen. That code and AuthScreensInitHandler.tsx:148 both guard on Navigation.isActiveRoute(ROUTES.SIGN_IN_MODAL), which only works because the RHP becomes the active route. So isActiveRoute('r/<roomID>') is false. Same for the thread variant (active route becomes r/<threadID>).

  3. So the new condition falls through and navigates. Link.ts:625-628 doesn't return, and Link.ts:647-648 computes forceReplace from routes.at(-1)?.name === NAVIGATORS.RIGHT_MODAL_NAVIGATOR — with the sign-in RHP on top it replaces the sign-in modal with the room, which is precisely the "public room slide is opened" symptom in #96346.

  4. Nothing earlier in the function stops it. isAuthenticated is a parameter captured from hasAuthToken() at call time, i.e. false while signed out, and it stays false in the closure after the anonymous token is minted — so Link.ts:620 doesn't fire. shouldSkipDeepLinkNavigation is a no-op on native, canAnonymousUserAccessRoute('r/<id>') is true, and the onboarding/HOME guards don't apply to an anonymous user. The flipped line is the sole gate.

  5. Why the old condition broke #101525: subscribe.ts:61-66 deliberately does not hand report deeplinks to react-navigation while unauthenticated — openReportFromDeepLink is the only thing that can do that navigation. Post-sign-in the active route is Home, so !isActiveRoute returned early and the deeplink was silently dropped.

Other findings

  1. iOS - Anonymous user does not land on a public room via link looks only partially addressed. That issue also reports the notification + Concierge modals appearing while logged out, a cut-off Sign in button, and an onboarding modal behind Home in split view. The first three are plausibly downstream of the missed redirect (anonymous session landing on the authenticated Home shell), but I found no code gating those modals on isAnonymousUser(), and "Sign in button is cut off" has no causal link to this condition at all. Please re-verify each symptom on device — if they survive, they're separate bugs and shouldn't be linked here.

  2. No test, in either PR. tests/unit/DeepLinkHandlerTest.tsx exists and neither scenario is encoded there. A two-case test — deeplinked report mounted in the tree vs. never mounted — would have caught the #101525 regression before staging, and would pin this line so it can't be flipped back and forth. The author checklist has "I added unit tests" checked, but the diff adds none.

  3. QA Steps is empty and still contains the // TODO: These must be filled out placeholder, while the title doesn't carry [No QA]. One or the other needs fixing before merge.

  4. Fixed Issues won't link the second issue. https://github.com/Expensify/App/issues/101551 is missing its $ prefix, so the automation will skip it.

  5. Screenshots cover only Android: Native, but both linked bugs are iOS-only (Reproducible in production?: No, iPhone 11). All platform boxes are checked in both checklists. Given #96346 reproduced differently on iOS vs Android, iOS evidence matters here.

  6. Nit — comment grammar: "Skip if the user already is in the deeplinked route" → "Skip if the user is already on the deeplinked route."

I was not able to verify any of this in a live browser: both flows start from a signed-out state, and signing out ends this session unrecoverably. Everything above is from source.


Next Steps: Reply with @MelvinBot <your feedback> to refine this review.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Regression hunt

Three regressions beyond the [$250] Public room-anon user first time tapping on sign in button or a thread open public room reopening I flagged earlier. Finding 1 is the important one — it's new behavior present in neither the pre-Fix: anonymous user first time tapping on sign in button or a thread open public room code nor current main.

The framing that fixes most of this: the guard is in the wrong place, not the wrong polarity. It sits at the top of navigateHandler, so it gates the not-found recovery as well as the navigate. Move it down to guard only the final Navigation.navigate(deeplinkRoute, …) at Link.ts:648 and findings 1 and 3 both disappear.

# Regression vs main vs pre-#99582
1 Report not-found / no-access fallback becomes unreachable on cold-start deeplinks
2 forceReplace revived — deeplink destroys an open RHP — (restores old behavior)
3 Deeplinks differing only by query params silently dropped
4 Two contradictory forceReplace predicates 9 lines apart pre-existing pre-existing

1. The not-found fallback is now dead on cold start ⬅️ new, worst

The guard is the first statement in navigateHandler, so returning early skips the recovery block right below it (Link.ts:633-644) that redirects to findLastAccessedReport or navigateToConciergeChat.

And "deeplink route is already active" is the normal cold-start case. This function already documents that, 75 lines up, in the secure-link branch — Link.ts:551-553:

// On cold launch the report is already the initial route; navigating again would stack a duplicate

React-navigation builds the nav state from the URL itself, and PublicScreens.tsx:33-34 deliberately registers SignInPage under NAVIGATORS.TAB_NAVIGATOR "so the public and auth root structures match" — so the URL-derived report route survives the sign-in swap and is focused by the time the handler runs.

Repro: signed out, force-close, tap a link to a report you can't access (deleted room, non-participant DM, or garbage /r/abc) → sign in → you're parked on "Hmm… it's not here" instead of being redirected to your last accessed report or Concierge. Both the old and the pre-#99582 code redirected you.

This also makes the deferred Onyx.connectWithoutView at Link.ts:652-665 pointless in the cold case — it exists specifically to wait for errorFields.notFound and then call navigateHandler, which now returns immediately.

Scope: all platforms on cold start. Warm native deeplinks (the iOS - Logged out user is not redirected to the correct conversation when navigate via link / iOS - Anonymous user does not land on a public room via link flows) still work, because subscribe.ts:65-67 drops report deeplinks there so nothing has navigated yet. Net effect: the flip moves the breakage from the warm flows to the cold flows.

2. forceReplace was dead code on main; this revives it

Link.ts:647-648 sets forceReplace when the top root route is RIGHT_MODAL_NAVIGATOR, which linkTo/index.ts:222-224 turns into a REPLACE — destroying the RHP. Under main's condition this was unreachable: an open RHP makes the RHP's path the active route, so !isActiveRoute(reportRoute) was always true and the handler always returned first. This PR makes it fire again.

That's the #96346 mechanism, but the reach is wider than the anonymous sign-in modal. Two other RHPs can be on top at that exact moment, both worth checking:

  • exitTo → RHP, e.g. a magic link v/:a/:c?exitTo=settings/…. handleExitToNavigation navigates at the same instant, and most settings/** destinations resolve into RIGHT_MODAL_NAVIGATOR.
  • The 3DS authorize-transaction RHP, auto-opened with zero user interaction at useNavigateTo3DSAuthorizationChallenge.ts:174 once TRANSACTIONS_PENDING_3DS_REVIEW arrives from OpenApp — i.e. a second or two after sign-in.

I confirmed the mechanism from code; I have not confirmed the timing of these two on a device.

3. Param-only deeplinks get dropped — and the guard is redundant with linkTo

isActiveRoute compares via cleanRoutePath, which strips everything after ?. That's asserted behavior, not an accident — isActiveRouteTests.tsx:33 expects isActiveRoute('settings/profile?param=1') === true while settings/profile?backTo=settings is active.

So the new early return treats r/<id>?referrer=…, search?q=…, attachment?source=…, and workspaces/<id>/join?email=… as "already there" and skips the navigate, losing the params. Bare r/<id> is guaranteed active on a cold-start sign-in, so r/<id>?… is the reachable one.

Worth noting: linkTo already dedupes the exact-same-route case, and does it params-aware — areNamesAndParamsEqual at linkTo/index.ts:62-70, called at :216. The new guard adds nothing there and is strictly coarser.

4. Nit: contradictory forceReplace predicates

Link.ts:639 uses Navigation.getTopmostReportId() === reportID; Link.ts:647 uses the RHP check. Nine lines apart, disagreeing on what "replace" means. Pre-existing, not introduced here, but it's in the blast radius of any fix.

Flows I checked and cleared

Not affected — all gated well before the flipped line:

  • Native OAuth callbackisNativeOAuthCallbackURL at Link.ts:533, before waitForUserSignIn().
  • Submit-via-PDF / secureKey — separate branch at Link.ts:542-561 that returns before navigateHandler.
  • transition links, Plaid iOS OAuth redirect, bare public screens, empty routeisPublicScreenRoute / shouldSkipDeepLinkNavigation / the explicit transition check.
  • ?exitTo=workspace/new, onboarding-incomplete users, route === ROUTES.HOME — all early-return above the guard.
  • Already-signed-in usersLink.ts:619-622 if (isAuthenticated) return. The whole blast radius is signed-out → sign-in plus anonymous cold start.
  • Onboarding / welcome / test-drive modals — separate root navigators, not RIGHT_MODAL_NAVIGATOR, so forceReplace stays false.

None of this is verifiable in a browser session here: Link.ts:620 makes the entire path unreachable while signed in, and signing out ends the test session unrecoverably. All findings are source-verified; the two device-timing caveats are called out inline.


Next Steps: Reply with @MelvinBot <your feedback> to refine this.


view run

@lakchote lakchote left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@lakchote
lakchote merged commit 6e383b7 into Expensify:main Sep 18, 2026
42 of 46 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify pushed a commit that referenced this pull request Sep 18, 2026
[CP Staging] fix deeplink issue

(cherry picked from commit 6e383b7)

(cherry-picked to staging by mountiny)
@OSBotify OSBotify added the CP Staging marks PRs that have been CP'd to staging label Sep 18, 2026
@os-botify

os-botify Bot commented Sep 18, 2026

Copy link
Copy Markdown

🔁 Filed a Staging retest request for deploy blockers #101525, #101551 after this PR was cherry-picked to staging.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.84-3 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Help site review: no changes required

No draft docs PR was created — nothing under App/docs/articles needs updating for this PR.

Why: this PR changes one line in src/libs/actions/Link.ts:625-628 — a navigation guard that decides whether a captured deep link is honored after sign-in. It's a bug fix that restores the already-expected behavior (tap a chat link while signed out → sign in → land on that chat). It adds no feature, setting, tab, button, or copy, and it doesn't change any documented workflow. The help site documents intended behavior, which this PR brings the app back in line with rather than redefining.

What I searched in App/docs/articles
Search Result
deep link / deeplink One hit — Connect-Cursor-to-Expensify-Using-MCP.md:31, a cursor:// install URL. Unrelated.
public room One hit — Chat-Features-and-Moderation-Tools.md:125, about moderation. Says nothing about anonymous access via a link.
sign in / log in / login Reviewed Login-Troubleshooting.md. Its only "link" reference is the unblock-email banner. Nothing describes where you land after signing in.
copy link / share the link / chat url / conversation link No article documents opening or sharing a chat URL, or the post-sign-in destination.

No article states the current (buggy) behavior, so there is nothing to correct; and no article would need to gain content, since the fixed behavior is what a reader would already assume.

Separately — this merged as [CP Staging] while my earlier review flagged that the condition flip re-opens [$250] Public room-anon user first time tapping on sign in button or a thread open public room. That's unrelated to the docs question, but worth confirming on staging before the next deploy.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.86-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Help site review: no changes required

No draft PR created — nothing under App/docs/articles needs updating for this change.

Why: the only file this PR touches is src/libs/actions/Link.ts:625-628 — a one-line guard inside openReportFromDeepLink that decides whether a deep link is still honored after sign-in. It adds no feature, setting, tab, or button, and changes no label or copy. It restores behavior users already expect (open a chat link while logged out → land on that chat after signing in) rather than introducing new behavior to document.

What I searched

I grepped all of App/docs/articles for content describing this flow. Three files matched and none is relevant:

The help site documents no deep link or anonymous public room navigation behavior that this change would contradict or make stale.

Since there's no help site PR, there's nothing for you to mark Ready for review. If you think a specific article should cover this, tell me which one and I'll draft the change.

Separately: this merged with the concern from my earlier review unaddressed — inverting the guard re-opens [$250] Public room-anon user first time tapping on sign in button or a thread open public room. Worth a follow-up.


Next Steps: Reply with @MelvinBot <your feedback> to refine this.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.89-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Help site review: no docs changes required

No draft PR created — nothing under App/docs/articles needs updating for this change.

Why: this PR changes one internal navigation guard in src/libs/actions/Link.ts:625-628 and nothing else. It's a bug fix that restores the already-documented-as-expected behavior (a shared chat link takes you to that chat after you sign in). It adds no feature, no UI element, no setting, no button, and no label — so there is no user-facing surface for an article to describe.

What I checked in App/docs/articles:

Search Result
deep link / deeplink 1 hit — Connect-Cursor-to-Expensify-Using-MCP.md, unrelated (MCP setup)
public room / anonymous 1 hit — Chat-Features-and-Moderation-Tools.md line 125, about anonymous moderation reporting, not anonymous link access
share link / copy link / conversation link / chat link Only Workspace-Rules.md and Create-a-company-workspace.md, neither about chat links
magic code No hits
All 6 files in new-expensify/chat/ None document opening a chat via a shared link, or what happens when you sign in from one

The help site simply doesn't document the sign-in-then-redirect flow or anonymous public-room link access anywhere, so there's no existing article that this fix makes stale and no gap that this fix creates. Since no help site PR exists, there's nothing to mark Ready for review.


⚠️ Separately — and more urgently than the docs question — please see my earlier review on this PR. Inverting this guard re-opens [$250] Public room-anon user first time tapping on sign in button or a thread open public room, which Fix: anonymous user first time tapping on sign in button or a thread open public room fixed yesterday. That comment includes a suggested condition that satisfies all three issues.


Next Steps: Reply with @MelvinBot <your feedback> if you think a specific article does need updating and I'll draft it.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/luacmartins in version: 9.4.89-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 failure ❌

Bundle Size Analysis (Sentry):

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.90-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor

🤖 Help site review: no changes required

No draft PR created — this change needs no help site updates.

This PR touches one guard condition in src/libs/actions/Link.ts:625-628 that decides whether a deeplink navigates after sign-in. It's an internal navigation fix: no new feature, no UI label, tab, setting, or button change, and no change to any workflow a member performs. The intended behavior — open a chat link and land on that chat — is what users already expect, so there is nothing new to document.

What I checked in docs/articles
  • Searched all of docs/articles for deep link / deeplink. One hit: Connect-Cursor-to-Expensify-Using-MCP.md, which uses the term for an MCP install link — unrelated to report deeplinks.
  • Searched for conversation link, chat link, link to a chat, copy link, share the link, room link — no articles document opening a chat from a shared link, signing in from a link, or the post-sign-in destination.
  • Searched for public room / anonymous. One hit: Chat-Features-and-Moderation-Tools.md:125, which covers moderation of public rooms, not anonymous access via a link.

There is no article whose stated behavior this PR contradicts or extends, so no update is warranted under HELP_AUTHORING_GUIDELINES.md — every article must solve one real user workflow, and there's no workflow change here.

Separately, please don't lose track of my earlier review on this PR: flipping this condition re-opens [$250] Public room-anon user first time tapping on sign in button or a thread open public room. That's unrelated to docs, but it still blocks this [CP Staging].

Since no help site PR was needed, there's nothing for you to review on that side.


view run

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/lakchote in version: 9.4.90-2 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CP Staging marks PRs that have been CP'd to staging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants