Fix deeplink handler

[enahum]

Summary

Deeplinks in the app where not handled correctly sending the user to a screen with unmatched route

Ticket Link

Fixes: #9862

Release Note

Fixed deeplink routing

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors native intent URL handling into a shared async helper and updates several redirect paths to preserve or normalize route parameters before navigation.

Changes

Native intent URL handling refactor

Layer / File(s)	Summary
Shared handleUrl helper and event listener wiring app/routes/+native-intent.ts	Adds url-parse, introduces a shared handleUrl helper that recognises SSO redirect schemes, parses other URLs, calls parseAndHandleDeepLink, and shows the invalid-deep-link alert on errors; addEventListener now uses this helper for Expo Linking URL events.
redirectSystemPath delegation app/routes/+native-intent.ts	Updates redirectSystemPath to delegate options.path handling to handleUrl, returning null when handled and the original path otherwise.

Redirect parameter conversion

Layer / File(s)	Summary
Route param conversion on redirects app/actions/remote/ephemeral_mode/reconnect.ts, app/managers/session_manager.ts, app/routes/(authenticated)/(home)/index.tsx, app/routes/index.tsx, app/actions/remote/ephemeral_mode/reconnect.test.ts	Applies propsToParams before redirecting in reconnect, logout, session-expired, and root redirect flows, forwards local search params in the home index redirect, and updates the reconnect success test to expect the converted coldStart value.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and directly reflects the main change: fixing deeplink handling.
Description check	✅ Passed	The description is related to the deeplink routing fix and references the linked bug report.
Linked Issues check	✅ Passed	The changes address deeplink routing and route parameter handling needed to avoid unmatched-route failures for the reported DM deeplink.
Out of Scope Changes check	✅ Passed	The additional navigation parameter conversions and tests are still tied to the deeplink fix and do not appear unrelated.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch deeplinks

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

app/routes/+native-intent.ts (1)

19-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove impossible URL guards and log the SSO no-op path.

event.url is typed as string, so the optional chaining, if (event.url), and final missing-URL return model an impossible state. Also, the SSO early return should emit a prefixed logDebug entry without logging the URL payload. As per coding guidelines, “Trust well-formed data - don't add defensive null checks if types guarantee existence” and “Log on early returns in handlers with a logDebug call so the no-op is traceable, and add function/class prefix to logs”.

Also applies to: 42-42

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/routes/`+native-intent.ts around lines 19 - 26, The handleUrl flow in
+native-intent.ts is over-defensive because event.url is already guaranteed to
be a string, so remove the optional chaining, the redundant if (event.url)
check, and any fallback path for a missing URL. Update the Sso redirect early
return in handleUrl to emit a prefixed logDebug entry that names the handler and
indicates the SSO no-op, but do not include the URL payload in that log. Keep
the logic centered on the handleUrl and Sso.REDIRECT_URL_SCHEME checks so the
intent path remains traceable without impossible null guards.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/routes/`+native-intent.ts:
- Around line 68-75: The deeplink handler in redirectSystemPath currently skips
handleUrl on cold starts because it only runs when options.initial is false,
which lets initial mattermost:// routes fall through to Expo Router. Update
redirectSystemPath to invoke handleUrl for both initial and non-initial paths,
and keep returning null whenever the URL is successfully consumed so unmatched
routes are avoided.

---

Nitpick comments:
In `@app/routes/`+native-intent.ts:
- Around line 19-26: The handleUrl flow in +native-intent.ts is over-defensive
because event.url is already guaranteed to be a string, so remove the optional
chaining, the redundant if (event.url) check, and any fallback path for a
missing URL. Update the Sso redirect early return in handleUrl to emit a
prefixed logDebug entry that names the handler and indicates the SSO no-op, but
do not include the URL payload in that log. Keep the logic centered on the
handleUrl and Sso.REDIRECT_URL_SCHEME checks so the intent path remains
traceable without impossible null guards.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5b88d77c-6caf-4853-bb17-0ed31e6946b3

📥 Commits

Reviewing files that changed from the base of the PR and between e18fc69 and 2842a66.

📒 Files selected for processing (1)

• app/routes/+native-intent.ts

[github-actions]

Coverage Comparison Report

Generated on July 09, 2026 at 02:09:02 UTC

+-----------------+------------+------------+-----------+
| Metric          | Main       | This PR    | Diff      |
+-----------------+------------+------------+-----------+
| Lines           |     87.77% |     87.79% |     0.02% |
| Statements      |     87.64% |     87.65% |     0.01% |
| Branches        |     76.59% |     76.60% |     0.01% |
| Functions       |     87.23% |     87.25% |     0.02% |
+-----------------+------------+------------+-----------+
| Total           |     84.80% |     84.82% |     0.02% |
+-----------------+------------+------------+-----------+

[coderabbitai]

🧹 Nitpick comments (1)

app/routes/+native-intent.test.ts (1)

16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use jest.mocked() for the fully-mocked @utils/deep_link functions.

@utils/deep_link is fully auto-mocked (jest.mock('@utils/deep_link')), so per guidelines, jest.mocked(parseAndHandleDeepLink) / jest.mocked(alertInvalidDeepLink) should be used instead of (parseAndHandleDeepLink as jest.Mock) casts throughout the file.

As per coding guidelines, "Use jest.mocked() for full implementations and (thing as jest.Mock) type assertion for partial implementations in tests."

Also applies to: 61-61, 116-116, 117-117

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/routes/`+native-intent.test.ts at line 16, The deep link test file is
using direct `as jest.Mock` casts on fully auto-mocked `@utils/deep_link`
exports; update the mocking calls to use `jest.mocked(parseAndHandleDeepLink)`
and `jest.mocked(alertInvalidDeepLink)` consistently in the affected test cases.
Keep the existing test behavior the same, but replace the casts in the
`+native-intent` test setup and assertions so the mocked functions are typed via
`jest.mocked()` instead of manual mock assertions.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/routes/`+native-intent.test.ts:
- Line 16: The deep link test file is using direct `as jest.Mock` casts on fully
auto-mocked `@utils/deep_link` exports; update the mocking calls to use
`jest.mocked(parseAndHandleDeepLink)` and `jest.mocked(alertInvalidDeepLink)`
consistently in the affected test cases. Keep the existing test behavior the
same, but replace the casts in the `+native-intent` test setup and assertions so
the mocked functions are typed via `jest.mocked()` instead of manual mock
assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 09a8e635-9c7b-4026-a100-ac3b2c12a409

📥 Commits

Reviewing files that changed from the base of the PR and between 2652062 and a2da833.

📒 Files selected for processing (6)

• app/actions/remote/ephemeral_mode/reconnect.test.ts
• app/actions/remote/ephemeral_mode/reconnect.ts
• app/managers/session_manager.ts
• app/routes/(authenticated)/(home)/index.tsx
• app/routes/+native-intent.test.ts
• app/routes/index.tsx

[carlisgg]

why do we need to use propsToParams here and other similar places in this PR? Is it in order to stringify non string values that is causing some issue?

[enahum]

Yes is needed as the router uses a query string and passing the object would be serialized to "[Object object]" this was an oversight on my part before.. you can see the navigation file, navigateToScreen where it first converts the props to params