Conversation
This commit adds `android:hasFragileUserData="true"` to the `AndroidManifest.xml`. This ensures that when a user uninstalls the app, the system prompts them to confirm if they want to also remove the app's data. The change also includes `tools:targetApi="29"` to support this attribute.
This commit enables the application to receive and process shared URLs (for `github.com` and `github-store.org`) from other apps through the Android Share Sheet. When a user shares text containing a supported URL with the app, the app will now extract the URL and navigate to the corresponding repository details screen. - **feat(android)**: Added an `intent-filter` for `ACTION_SEND` in `AndroidManifest.xml` to accept `text/plain` and `text/html` content. - **feat(deeplink)**: Implemented `DeepLinkParser.extractSupportedUrl()` to find the first `github.com` or `github-store.org` URL within a given text string. - **refactor(android)**: Updated `MainActivity` to handle `ACTION_SEND` intents, extract the URL from the shared text, and trigger the deep link flow. - **fix(android)**: Corrected the `pathPattern` in the `AndroidManifest.xml` for `github.com` deep links to correctly match repository paths.
Contributor
WalkthroughThe changes add deep link handling for receiving shared content via intent, implement centralized intent processing in MainActivity, extract supported URLs from shared text, and expand Android manifest declarations with security attributes and intent filters for both ACTION_VIEW and ACTION_SEND. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Android as Android System
participant MA as MainActivity
participant DLP as DeepLinkParser
participant App as App/Main
User->>Android: Share text via ACTION_SEND
Android->>MA: handleIncomingIntent(intent)
MA->>MA: Extract EXTRA_TEXT from intent
MA->>DLP: extractSupportedUrl(text)
DLP->>DLP: Regex match for github.com/<br/>github-store.org
DLP-->>MA: Return matched URL or null
alt URL found
MA->>MA: Parse URL via DeepLinkParser
MA->>MA: Assign result to deepLinkUri
MA->>App: Trigger deep link navigation
else URL not found
MA->>MA: deepLinkUri remains unchanged
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/deeplink/DeepLinkParser.kt (1)
178-181: Regex matches spoofed domains and captures trailing punctuation.Two issues with the URL extraction pattern:
Domain boundary missing: The pattern matches
https://github.com.evil.com/pathbecause there's no boundary after the domain literal. Whileparse()downstream mitigates this (it checks forhttps://github.com/with the trailing slash), it's better to be precise at extraction time.Trailing punctuation captured: If the shared text is
"Check out https://github.com/owner/repo.", the trailing.is included in the match. Same for),,,;, etc. — common in natural-language messages. This can causeparse()to fail on otherwise valid URLs.Proposed fix
- fun extractSupportedUrl(text: String): String? { - val regex = """https?://(?:www\.)?(?:github\.com|github-store\.org)[^\s<>"']+""".toRegex(RegexOption.IGNORE_CASE) - return regex.find(text)?.value - } + private val supportedUrlRegex = + """https?://(?:www\.)?(?:github\.com|github-store\.org)(?:/[^\s<>"')\],;.!?]*)?""".toRegex(RegexOption.IGNORE_CASE) + + fun extractSupportedUrl(text: String): String? { + return supportedUrlRegex.find(text)?.value?.trimEnd('/', '.') + }The revised pattern:
- Requires
/(or end) after the domain, preventing subdomain spoofing.- Excludes common sentence-ending punctuation from the character class.
- Compiles the
Regexonce as a property instead of on every call.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/deeplink/DeepLinkParser.kt` around lines 178 - 181, The extractSupportedUrl function uses a regex that allows spoofed domains (e.g., github.com.evil.com) and captures trailing punctuation; update it to a precompiled Regex property (not recreated on each call) that requires either a slash or end-of-string after the domain portion (so the domain literal is a boundary) and limits the subsequent character class to exclude common sentence punctuation like . , ; : ) ] > ' " so trailing punctuation isn't captured; replace the inline toRegex usage in extractSupportedUrl to use this new precompiled pattern and return its first match value.composeApp/src/androidMain/AndroidManifest.xml (1)
74-76: VerifypathPatterngranularity with split<data>elements.When
scheme,host, andpathPatternare declared in separate<data>tags inside the same<intent-filter>, Android combines them multiplicatively. With a single value each this works correctly, but it's a subtle rule that can cause surprising matches if more<data>entries are added later. Keeping them in a single<data>tag (if the manifest schema permits) or adding a comment documenting the multiplicative combination would prevent accidental regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/androidMain/AndroidManifest.xml` around lines 74 - 76, The three separate <data> elements inside the same <intent-filter> (the ones declaring android:scheme="https", android:host="github.com", and android:pathPattern="/.*/.*") will be combined multiplicatively by Android and can lead to surprising matches if more <data> entries are added later; fix this by consolidating these attributes into a single <data> entry with android:scheme, android:host and android:pathPattern together (or, if you must keep separate tags, add a clear comment above the intent-filter explaining the multiplicative combination rule) so the intent-filter semantics are explicit and safe from accidental regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composeApp/src/androidMain/AndroidManifest.xml`:
- Around line 61-66: The manifest's SEND intent-filter with text/plain and
text/html makes the app appear for all text shares; update handleIncomingIntent
to detect when an ACTION_SEND intent yields no supported URL (i.e.,
extractSupportedUrl(...) returns null) and show a brief user-visible message
(Toast or Snackbar) explaining the share was ignored; locate the code that
handles Intent.ACTION_SEND in handleIncomingIntent and add a concise
Toast/Snackbar path for the null result case so users get feedback instead of
silent no-op.
In `@composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/MainActivity.kt`:
- Around line 45-60: The incoming-intent handler (handleIncomingIntent) sets
deepLinkUri but because App/Main.kt never clears it, a repeated identical share
won't retrigger LaunchedEffect(deepLinkUri); change the flow so each consumed
deepLink signals back to clear or produce a unique wrapper: after navigation in
Main.kt's LaunchedEffect that reacts to deepLinkUri, reset deepLinkUri to null
(or call a provided clearDeepLink callback), or alternatively change deepLinkUri
to a wrapper (e.g., a data class/Pair with an incrementing nonce) and update
handleIncomingIntent to set a new unique wrapper on each intent; also update the
LaunchedEffect key to use the wrapper (or pair) and ensure
handleIncomingIntent/DeepLinkParser/App agree on the clear/reset contract.
---
Nitpick comments:
In `@composeApp/src/androidMain/AndroidManifest.xml`:
- Around line 74-76: The three separate <data> elements inside the same
<intent-filter> (the ones declaring android:scheme="https",
android:host="github.com", and android:pathPattern="/.*/.*") will be combined
multiplicatively by Android and can lead to surprising matches if more <data>
entries are added later; fix this by consolidating these attributes into a
single <data> entry with android:scheme, android:host and android:pathPattern
together (or, if you must keep separate tags, add a clear comment above the
intent-filter explaining the multiplicative combination rule) so the
intent-filter semantics are explicit and safe from accidental regressions.
In
`@composeApp/src/commonMain/kotlin/zed/rainxch/githubstore/app/deeplink/DeepLinkParser.kt`:
- Around line 178-181: The extractSupportedUrl function uses a regex that allows
spoofed domains (e.g., github.com.evil.com) and captures trailing punctuation;
update it to a precompiled Regex property (not recreated on each call) that
requires either a slash or end-of-string after the domain portion (so the domain
literal is a boundary) and limits the subsequent character class to exclude
common sentence punctuation like . , ; : ) ] > ' " so trailing punctuation isn't
captured; replace the inline toRegex usage in extractSupportedUrl to use this
new precompiled pattern and return its first match value.
Comment on lines
+61
to
+66
| <intent-filter> | ||
| <action android:name="android.intent.action.SEND" /> | ||
| <category android:name="android.intent.category.DEFAULT" /> | ||
| <data android:mimeType="text/plain" /> | ||
| <data android:mimeType="text/html" /> | ||
| </intent-filter> |
Contributor
There was a problem hiding this comment.
App will appear in the share sheet for all plain/HTML text shares.
The SEND intent-filter with text/plain and text/html cannot be scoped to specific domains at the manifest level, so the app shows up whenever a user shares any text — articles, notes, non-GitHub links, etc. When the shared text doesn't contain a supported URL, extractSupportedUrl returns null and nothing happens, but the user gets no feedback explaining why.
Consider adding a brief Toast or Snackbar in handleIncomingIntent when ACTION_SEND yields no supported URL, so the user understands the share was intentionally ignored.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composeApp/src/androidMain/AndroidManifest.xml` around lines 61 - 66, The
manifest's SEND intent-filter with text/plain and text/html makes the app appear
for all text shares; update handleIncomingIntent to detect when an ACTION_SEND
intent yields no supported URL (i.e., extractSupportedUrl(...) returns null) and
show a brief user-visible message (Toast or Snackbar) explaining the share was
ignored; locate the code that handles Intent.ACTION_SEND in handleIncomingIntent
and add a concise Toast/Snackbar path for the null result case so users get
feedback instead of silent no-op.
Comment on lines
+45
to
+60
| private fun handleIncomingIntent(intent: Intent?) { | ||
| if (intent == null) return | ||
|
|
||
| val uriString = when (intent.action) { | ||
| Intent.ACTION_VIEW -> intent.data?.toString() | ||
|
|
||
| Intent.ACTION_SEND -> { | ||
| val sharedText = intent.getStringExtra(Intent.EXTRA_TEXT) | ||
| sharedText?.let { DeepLinkParser.extractSupportedUrl(it) } | ||
| } | ||
|
|
||
| else -> null | ||
| } | ||
|
|
||
| uriString?.let { deepLinkUri = it } | ||
| } |
Contributor
There was a problem hiding this comment.
Good centralized handling; but re-sharing the same URL is silently ignored.
deepLinkUri is never reset after consumption in App(). If the user shares the exact same URL a second time, the value doesn't change, so LaunchedEffect(deepLinkUri) in Main.kt won't re-fire — the share appears to do nothing.
A simple fix is to reset the URI after navigation and use a wrapper to ensure uniqueness:
Sketch: reset after consumption
In Main.kt, reset deepLinkUri after navigating:
// After navigating in LaunchedEffect:
// signal back to clear the deep link so a repeated share is recognizedOr in MainActivity.kt, wrap the value so each intent is unique:
- private var deepLinkUri by mutableStateOf<String?>(null)
+ private var deepLinkUri by mutableStateOf<Pair<String, Long>?>(null)- uriString?.let { deepLinkUri = it }
+ uriString?.let { deepLinkUri = it to System.currentTimeMillis() }Then key LaunchedEffect on the pair.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composeApp/src/androidMain/kotlin/zed/rainxch/githubstore/MainActivity.kt`
around lines 45 - 60, The incoming-intent handler (handleIncomingIntent) sets
deepLinkUri but because App/Main.kt never clears it, a repeated identical share
won't retrigger LaunchedEffect(deepLinkUri); change the flow so each consumed
deepLink signals back to clear or produce a unique wrapper: after navigation in
Main.kt's LaunchedEffect that reacts to deepLinkUri, reset deepLinkUri to null
(or call a provided clearDeepLink callback), or alternatively change deepLinkUri
to a wrapper (e.g., a data class/Pair with an incrementing nonce) and update
handleIncomingIntent to set a new unique wrapper on each intent; also update the
LaunchedEffect key to use the wrapper (or pair) and ensure
handleIncomingIntent/DeepLinkParser/App agree on the clear/reset contract.
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.
Summary by CodeRabbit