Improve the Quickstart sample#2
Merged
Merged
Conversation
brionmario
force-pushed
the
android-sdk
branch
4 times, most recently
from
486ba1d to
92a7ddd
Compare
brionmario
changed the title
brionmario
force-pushed
the
android-sdk
branch
7 times, most recently
from
d6d083c to
d862afc
Compare
📝 WalkthroughWalkthroughThis PR renames IAM error types to ThunderIDErrorCode, adds insecure TLS support to HttpClient, changes token/session establishment to save assertions directly, and updates the quickstart sample, Compose components, and release/build tooling. ChangesRepository tooling and build configuration
Quickstart sample app updates
SDK core client, error codes, HTTP, and token handling
Compose component library updates
Release automation
Estimated code review effort: 4 (Complex) | ~75 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Wire up the ThunderID launcher icon across all mipmap densities and reference it from the manifest. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/main/kotlin/dev/thunderid/compose/components/guards/SignedIn.kt (1)
30-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
SignedIntreats the initialization window as signed out.isSignedInstays false untilThunderIDProviderfinishesinitialize(), sofallback()can briefly show before auth state is known. Gate this onisInitialized/isLoading, or document that callers should wrap it withLoading.🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/guards/SignedIn.kt` around lines 30 - 31, The SignedIn guard currently falls back whenever isSignedIn is false, which causes a brief signed-out state during ThunderIDProvider initialization. Update SignedIn to also check the initialization/loading state from LocalThunderID (for example, using isInitialized or isLoading) before calling fallback(), or otherwise make the intended Loading-wrapper requirement explicit in the guard’s behavior using the SignedIn symbol.src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt (1)
206-216: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNormalize malformed JWT payload failures to
IAMException.A 3-part token with invalid Base64 or JSON still throws platform exceptions instead of the typed SDK error. As per coding guidelines, SDK errors should use
ThunderIDErrorCodeandIAMException.Suggested fix
- val json = String(android.util.Base64.decode(padded, android.util.Base64.DEFAULT), Charsets.UTF_8) - return org.json.JSONObject(json).let { obj -> - obj.keys().asSequence().associateWith { obj.opt(it) } - } + val json = + runCatching { + String(android.util.Base64.decode(padded, android.util.Base64.DEFAULT), Charsets.UTF_8) + }.getOrElse { + throw IAMException(ThunderIDErrorCode.INVALID_INPUT, "Invalid JWT payload") + } + return runCatching { + org.json.JSONObject(json).let { obj -> + obj.keys().asSequence().associateWith { obj.opt(it) } + } + }.getOrElse { + throw IAMException(ThunderIDErrorCode.INVALID_INPUT, "Invalid JWT payload") + }🤖 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 `@src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt` around lines 206 - 216, Normalize all JWT payload parsing failures in decodeJwtToken to IAMException by catching Base64/JSON parsing exceptions around the payload decode and JSONObject creation. Keep the existing INVALID_INPUT path for bad token shape, and map malformed payload content to ThunderIDErrorCode.INVALID_INPUT with a clear message instead of letting platform exceptions escape. Use the decodeJwtToken function as the single entry point for this typed error handling.Source: Coding guidelines
samples/quickstart/build.gradle.kts (1)
24-34: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUnescaped values interpolated into
buildConfigFieldsource.
config(key)values are spliced directly into a Kotlin string literal ("\"$baseUrl\""). If any property/env value contains a"or\, it will break the generatedBuildConfigsource (or worse, inject arbitrary code, sincebuildConfigField's third argument is raw source, not an escaped string).🛡️ Proposed fix to escape special characters
fun config(key: String) = configProps.getProperty(key) ?: System.getenv(key) ?: "" + fun escaped(value: String) = value.replace("\\", "\\\\").replace("\"", "\\\"") val baseUrl = config("THUNDERID_BASE_URL") val clientId = config("THUNDERID_CLIENT_ID") val appId = config("THUNDERID_APPLICATION_ID") val afterSignInUrl = config("THUNDERID_AFTER_SIGN_IN_URL") val afterSignOutUrl = config("THUNDERID_AFTER_SIGN_OUT_URL") - buildConfigField("String", "THUNDERID_BASE_URL", "\"$baseUrl\"") - buildConfigField("String", "THUNDERID_CLIENT_ID", "\"$clientId\"") - buildConfigField("String", "THUNDERID_APPLICATION_ID", "\"$appId\"") - buildConfigField("String", "THUNDERID_AFTER_SIGN_IN_URL", "\"$afterSignInUrl\"") - buildConfigField("String", "THUNDERID_AFTER_SIGN_OUT_URL", "\"$afterSignOutUrl\"") + buildConfigField("String", "THUNDERID_BASE_URL", "\"${escaped(baseUrl)}\"") + buildConfigField("String", "THUNDERID_CLIENT_ID", "\"${escaped(clientId)}\"") + buildConfigField("String", "THUNDERID_APPLICATION_ID", "\"${escaped(appId)}\"") + buildConfigField("String", "THUNDERID_AFTER_SIGN_IN_URL", "\"${escaped(afterSignInUrl)}\"") + buildConfigField("String", "THUNDERID_AFTER_SIGN_OUT_URL", "\"${escaped(afterSignOutUrl)}\"")🤖 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 `@samples/quickstart/build.gradle.kts` around lines 24 - 34, The values returned by config() are being interpolated directly into buildConfigField as raw source, so unescaped quotes or backslashes can break the generated BuildConfig or inject invalid code. Update the quickstart build script around config() and the THUNDERID_* buildConfigField calls to escape these values before wrapping them in Kotlin string literals, or use a safe helper that produces properly escaped source for BuildConfig fields.src/test/kotlin/dev/thunderid/android/ThunderIDClientTest.kt (1)
121-136: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winTrailing comma on a call-site
TokenResponse(...)constructor invocation.
idToken = "id789",at Line 130 is the last argument of a call-site invocation (constructingTokenResponse). As per coding guidelines,Do not use a trailing comma on the last argument at call sites.🔧 Proposed fix
val response = TokenResponse( accessToken = "access123", tokenType = "Bearer", expiresIn = 3600, refreshToken = "refresh456", - idToken = "id789", + idToken = "id789" )🤖 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 `@src/test/kotlin/dev/thunderid/android/ThunderIDClientTest.kt` around lines 121 - 136, The TokenResponse constructor call in ThunderIDClientTest uses a trailing comma on the last call-site argument, which violates the coding guideline. Update the TokenResponse invocation in TokenStore saves and retrieves tokens so the final argument is written without a trailing comma, keeping the rest of the test unchanged.Source: Coding guidelines
src/main/kotlin/dev/thunderid/compose/components/presentation/user/User.kt (2)
30-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude email before falling back to anonymous.
Users with no
displayName/usernamebut a populatedsrc/main/kotlin/dev/thunderid/android/Models.ktandsrc/main/kotlin/dev/thunderid/compose/components/presentation/user/UserDropdown.kt.🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/user/User.kt` around lines 30 - 38, Update UserObject so its identity fallback includes email before using the anonymous i18n string. In the UserObject composable, adjust the label selection order to use user.displayName, then user.username, then user.email, and only then i18n.resolve("user.anonymous"), matching the fallback behavior used in UserDropdown and the user model.
41-48: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply the passed modifier.
modifieris never used here, so callers ofUserObject(modifier = ...)lose layout/semantics control. Wrap the content in a container that forwardsmodifier.🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/user/User.kt` around lines 41 - 48, The BaseUserObject composable ignores its modifier parameter, so callers cannot control layout or semantics. Update BaseUserObject to wrap the content lambda in a container that accepts and forwards the passed modifier, using the BaseUserObject symbol as the place to apply it while still passing state.user into content.
🟡 Minor comments (12)
AGENTS.md-47-67 (1)
47-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLabel the fenced directory-tree block.
markdownlintflags unlabeled fences here. Usetextso the docs still render cleanly and the lint rule passes.Suggested edit
-``` +```text🤖 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 `@AGENTS.md` around lines 47 - 67, The fenced directory-tree block in AGENTS.md is unlabeled, which triggers markdownlint. Update that existing markdown fence to use a text label so it renders correctly and passes linting, keeping the directory tree content unchanged.Source: Linters/SAST tools
src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt-52-57 (1)
52-57: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the remaining ktlint-sensitive formatting.
Several changed call sites keep trailing commas, and Lines 89, 115, 202, 289, and 290 exceed the 120-character limit. This can keep the CI formatting fix incomplete. As per coding guidelines, Kotlin files must pass
ktlintCheck, keep lines to 120 characters or fewer, and avoid trailing commas at call sites.Also applies to: 89-89, 115-115, 142-148, 202-202, 221-226, 289-290, 301-307
🤖 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 `@src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt` around lines 52 - 57, The remaining ktlint issues in ThunderIDClient should be cleaned up by removing trailing commas from the affected call sites and reflowing any overlong expressions so no Kotlin line exceeds 120 characters. Update the relevant sections in ThunderIDClient and the related call sites around the referenced symbols/blocks to match ktlint formatting rules, then rerun ktlintCheck to confirm the file is fully clean.Source: Coding guidelines
src/main/kotlin/dev/thunderid/android/http/HttpClient.kt-21-34 (1)
21-34: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClean up the remaining ktlint violations.
Imports are not alphabetically sorted, and Line 103 exceeds the 120-character limit. As per coding guidelines, Kotlin files must pass
ktlintCheck, sort imports alphabetically, and keep lines to 120 characters or fewer.Also applies to: 103-103
🤖 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 `@src/main/kotlin/dev/thunderid/android/http/HttpClient.kt` around lines 21 - 34, The Kotlin file still has ktlint issues: the import block in HttpClient must be alphabetized, and the long statement in the HttpClient logic around the line that currently exceeds 120 characters needs to be wrapped. Update the relevant imports in HttpClient.kt so they are sorted consistently, and refactor the long expression in the affected method to fit within the line-length limit while preserving behavior.Source: Coding guidelines
samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt-720-730 (1)
720-730: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSwallowed exception flagged by detekt.
eis caught but never used/logged, tripping detekt'sSwallowedExceptionrule.🔇 Suppress via detekt's allowed-name convention
- } catch (e: Exception) { + } catch (_: Exception) { emptyMap() }Based on detekt's default config,
allowedExceptionNameRegexmatches_, so renaming the caught variable resolves this without adding logging.🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt` around lines 720 - 730, The caught exception in decodeJwtPayload is intentionally unused, so it triggers detekt’s SwallowedException rule. Rename the caught Exception parameter to the allowed underscore convention in decodeJwtPayload and keep the empty fallback behavior unchanged; this satisfies the default allowedExceptionNameRegex without adding logging or altering the return path.Source: Linters/SAST tools
samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt-70-75 (1)
70-75: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winImport order:
java.*should precedekotlinx.*.
java.text.SimpleDateFormat/java.util.*(lines 73-75) come afterkotlinx.coroutines.*(lines 70-72), breaking alphabetical import order ('j' < 'k').🔤 Fix import order
-import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.delay -import kotlinx.coroutines.withContext -import java.text.SimpleDateFormat -import java.util.Date -import java.util.Locale +import java.text.SimpleDateFormat +import java.util.Date +import java.util.Locale +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay +import kotlinx.coroutines.withContextAs per coding guidelines, "Sort imports alphabetically and do not use wildcard imports."
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt` around lines 70 - 75, The import block is out of alphabetical order in HomeScreen, with java.* imports appearing after kotlinx.* imports. Reorder the imports so java.text.SimpleDateFormat and java.util.Date/Locale come before kotlinx.coroutines.Dispatchers, delay, and withContext, and keep the list alphabetized consistently.Source: Coding guidelines
samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/MainActivity.kt-44-45 (1)
44-45: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove trailing comma at this call site.
ThunderIDConfig(...)is a call site; per this repo's ktlint guideline, trailing commas are disallowed there (only allowed on multi-line declarations). Line 45's trailing comma before the closing)violates this and would failktlintCheck, which is the exact class of issue this PR is meant to fix.🔧 Proposed fix
storage = EncryptedStorageAdapter(this), - allowInsecureConnections = BuildConfig.DEBUG, + allowInsecureConnections = BuildConfig.DEBUG )Based on coding guidelines: "Do not use a trailing comma on the last argument at call sites."
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/MainActivity.kt` around lines 44 - 45, Remove the trailing comma at the ThunderIDConfig call site in MainActivity, since ktlint forbids trailing commas on the last argument in call expressions. Update the argument list around the ThunderIDConfig(...) invocation so allowInsecureConnections is the final argument without a comma, keeping the rest of the call unchanged.Source: Coding guidelines
samples/quickstart/src/main/AndroidManifest.xml-7-8 (1)
7-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
android:networkSecurityConfigto<application>.samples/quickstart/src/main/res/xml/network_security_config.xmlis present, butsamples/quickstart/src/main/AndroidManifest.xmlnever references it, so the debug trust anchors won’t take effect.🤖 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 `@samples/quickstart/src/main/AndroidManifest.xml` around lines 7 - 8, The <application> section in AndroidManifest is missing the network security config reference, so the debug trust anchors from network_security_config.xml are never applied. Update the <application> attributes in AndroidManifest to include android:networkSecurityConfig pointing at the existing network_security_config.xml resource, keeping the change scoped to the quickstart app’s manifest.src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt-104-110 (1)
104-110: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTrailing commas at call sites violate the project's ktlint policy.
BasicTextField(...)(line 109.semantics { contentDescription = input.name },) andEmbeddedSignInPayload(...)(line 144challengeToken = signUpState.challengeToken,) both end with a trailing comma on the last argument at a call site.As per coding guidelines: "Do not use a trailing comma on the last argument at call sites." (declarations should have trailing commas; call sites should not).
🧹 Proposed fix
modifier = Modifier.fillMaxWidth().defaultMinSize(minHeight = 44.dp) - .semantics { contentDescription = input.name }, + .semantics { contentDescription = input.name } )- challengeToken = signUpState.challengeToken, + challengeToken = signUpState.challengeToken )Also applies to: 139-145
🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt` around lines 104 - 110, Remove the trailing commas from the last arguments at call sites to match ktlint policy. Update the `BasicTextField` call in `SignUp.kt` so the `modifier` argument no longer ends with a comma, and do the same for the `EmbeddedSignInPayload` construction so `challengeToken = signUpState.challengeToken` is the final argument without a trailing comma.Source: Coding guidelines
src/test/kotlin/dev/thunderid/compose/ComponentTests.kt-50-58 (1)
50-58: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTrailing commas at call sites.
ThunderIDI18n(...)(lines 53-54, 66-67) andlistOf(...)(line 86-87) each keep a trailing comma on the last argument at a call site. Per coding guidelines, trailing commas are required on declaration sites but must not be used at call sites.🧹 Proposed fix
ThunderIDI18n( bundles = mapOf("fr-FR" to mapOf("signIn.button" to "Se connecter")), - language = "en-US", + language = "en-US" )ThunderIDI18n( bundles = mapOf("es-ES" to mapOf("signIn.button" to "Iniciar sesión")), language = "de-DE", - fallbackLanguage = "es-ES", + fallbackLanguage = "es-ES" )"organizationList.empty", "createOrganization.submit", - "languageSwitcher.title", "acceptInvite.submit", "inviteUser.submit", + "languageSwitcher.title", "acceptInvite.submit", "inviteUser.submit" )Also applies to: 61-68, 81-87
🤖 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 `@src/test/kotlin/dev/thunderid/compose/ComponentTests.kt` around lines 50 - 58, Remove the trailing commas from the call-site argument lists in ComponentTests, including the ThunderIDI18n constructor calls and the listOf usage referenced by the review. Keep the declarations unchanged, but update these invocation sites so the last argument in each call no longer ends with a comma while preserving the existing test behavior.Source: Coding guidelines
src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt-110-120 (1)
110-120: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTrailing commas at call sites violate the project's ktlint policy.
OutlinedTextField(...)(line 119singleLine = true,),Button(...)(line 126modifier = Modifier.fillMaxWidth(),), andEmbeddedSignInPayload(...)(line 156challengeToken = signInState.challengeToken,) each end with a trailing comma on the last argument at a call site.As per coding guidelines: "Use a trailing comma on the last element of multi-line declarations such as function parameters and enum entries." and "Do not use a trailing comma on the last argument at call sites." — these are call sites, not declarations, so the trailing commas should be removed to keep
ktlintCheckat zero violations.🧹 Proposed fix
- singleLine = true, + singleLine = true )- modifier = Modifier.fillMaxWidth(), + modifier = Modifier.fillMaxWidth() ) {- challengeToken = signInState.challengeToken, + challengeToken = signInState.challengeToken )Also applies to: 122-130, 153-157
🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt` around lines 110 - 120, Trailing commas are present on the last argument of several call sites and violate ktlint rules. Remove the trailing comma from the final parameter in the `OutlinedTextField`, `Button`, and `EmbeddedSignInPayload` invocations in `SignIn.kt`, keeping trailing commas only for multi-line declarations, not function calls.Source: Coding guidelines
src/main/kotlin/dev/thunderid/compose/i18n/DefaultStrings.kt-23-57 (1)
23-57: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTrailing comma at a call site.
mapOf(...)is a function call, not a declaration; per coding guidelines "Do not use a trailing comma on the last argument at call sites," the trailing comma after the final entry (line 56) should be removed.🧹 Proposed fix
- "inviteUser.loading" to "Sending…", + "inviteUser.loading" to "Sending…" )🤖 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 `@src/main/kotlin/dev/thunderid/compose/i18n/DefaultStrings.kt` around lines 23 - 57, The mapOf call in DefaultStrings.all uses a trailing comma on its last entry, which violates the call-site formatting rule. Remove the trailing comma after the final inviteUser.loading entry while keeping the rest of the string map unchanged. Use the DefaultStrings.all property and the mapOf builder as the place to make this cleanup.Source: Coding guidelines
src/main/kotlin/dev/thunderid/compose/components/presentation/user/UserProfile.kt-99-114 (1)
99-114: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep load errors non-null.
e.messagecan be null, which means this failure path can fall through to the edit form with an emptyerror. Use a fallback string so load failures always hit the error branch.🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/user/UserProfile.kt` around lines 99 - 114, The load failure handling in UserProfile’s profile-fetch path can store a null error because it assigns e.message directly. Update the catch block around thunderState.client.getUserProfile() to always set a non-null fallback message when the exception has no message, so the error state reliably triggers instead of falling through to the edit form.
🧹 Nitpick comments (7)
settings.gradle.kts (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGeneric
rootProject.namereduces project clarity.Renaming from
"thunderid-android"to"android"is a very generic name for a repo containing multiple modules (SDK, Compose library, quickstart sample). This can be confusing in IDE project trees, build logs, and CI artifact naming.♻️ Suggested rename
-rootProject.name = "android" +rootProject.name = "thunderid-android"🤖 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 `@settings.gradle.kts` at line 9, The root project name is too generic and makes the multi-module repo harder to पहचान in IDEs, build logs, and CI artifacts. Update the rootProject.name assignment in settings.gradle.kts back to a more specific project name like the original repository name, keeping the change localized to the root project identity so all modules inherit a clearer label.samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt (3)
404-413: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBack-navigation glyphs lack accessible labels.
The "‹ Home" back rows are plain
Text/Rowwithclickable, so screen readers have no meaningful announcement (e.g. "Back") for the glyph.♿ Add semantics
Row( modifier = Modifier .padding(horizontal = 24.dp) - .clickable(onClick = onBack), + .clickable(onClick = onBack) + .semantics { contentDescription = "Back to home" }, verticalAlignment = Alignment.CenterVertically, ) {Also applies to: 573-582
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt` around lines 404 - 413, The back-navigation row in HomeScreen uses plain Text inside a clickable Row, so it lacks an accessible announcement for screen readers. Update the back row(s) built with the clickable Row/“‹ Home” text to include semantics with a meaningful content description or role so assistive tech announces it as a back action; apply the same fix to the other matching back row referenced in the comment.
542-562: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHand-rolled JSON parsing/regex claim extraction instead of a JSON library.
prettyPrintJsonmanually walks characters (naively, without honoring escaped quotes\"inside string values, which would mis-toggleinString), andexpiryText/issuer/scopesre-extract claims via regex on the raw JSON string instead of reusing a parsed structure.org.json.JSONObject(built into Android, no new dependency) would be simpler and correctly handle escaping.Also applies to: 732-767
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt` around lines 542 - 562, The claim extraction logic in HomeScreen is manually parsing JSON with regex and a hand-rolled formatter, which is brittle and duplicates work. Replace the raw string parsing in prettyPrintJson and the expiryText/issuer/scopes remember blocks with a proper JSON parse using org.json.JSONObject, then read exp, iss, and scope from the parsed object instead of re-extracting from rawJson. Keep the existing UI values and fallback behavior, but centralize the JSON handling so escaped quotes and other valid JSON edge cases are handled correctly.
1-813: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftFile exceeds the ~400-line guideline for new files.
HomeScreen.ktspans to over 800 lines, combiningHomeTab,ProfileScreen,TokenDebugScreen, and all their helpers in one file. Consider splitting intoProfileScreen.kt,TokenDebugScreen.kt, and a shared UI-helpers file.Based on learnings, "Keep new files under roughly 400 lines by splitting large files when appropriate."
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt` around lines 1 - 813, The HomeScreen file is far over the new-file size guideline because it combines HomeScreen, HomeTab, ProfileScreen, TokenDebugScreen, and all shared helpers in one place. Split the UI into separate composables/files by feature: move ProfileScreen-related code into a ProfileScreen file, TokenDebugScreen into its own file, and extract shared helpers like StatColumn, StepRow, ActionRow, SectionHeader, DetailCard, DetailRow, and the JWT/date/user utility functions into a shared UI/helpers file. Keep HomeScreen focused on navigation and wiring, and update imports/usages so the top-level file stays well under the ~400-line limit.Source: Learnings
src/main/kotlin/dev/thunderid/android/ThunderIDConfig.kt (1)
46-49: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider guarding
allowInsecureConnectionsbeyond a doc comment.This flag flows directly into
HttpClient's trust-all TLS bypass (per the linkedHttpClient.kt/ThunderIDClient.ktsnippets). Right now the only protection against production misuse is the comment. Consider adding a runtime guard (e.g., assertBuildConfig.DEBUGor emit a prominent warning log) so a mis-settruevalue in a release config doesn't silently disable certificate validation.🤖 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 `@src/main/kotlin/dev/thunderid/android/ThunderIDConfig.kt` around lines 46 - 49, The allowInsecureConnections flag in ThunderIDConfig currently relies only on a comment, but it is used by ThunderIDClient/HttpClient to bypass TLS verification. Add a runtime safeguard where the flag is consumed, such as checking BuildConfig.DEBUG before enabling the trust-all path or logging a strong warning and refusing the insecure mode in non-debug builds. Keep the guard close to the HttpClient setup so a true value cannot silently disable certificate validation in release configurations.src/main/kotlin/dev/thunderid/android/ThunderIDError.kt (1)
57-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffRebrand is incomplete:
IAMExceptionretains old naming.
ThunderIDErrorCodewas renamed for the ThunderID rebrand, butIAMException(which wraps it) was not. This leaves a mixed public API (IAMException(ThunderIDErrorCode.X, ...)) that's used throughout the SDK (HttpClient, ThunderIDClient, etc.). As per coding guidelines,src/main/kotlin/dev/thunderid/android/**/*.ktshould "Use the typedIAMErrorCodeenum andIAMExceptionfor error handling" — that guideline itself references the now-removedIAMErrorCodename, suggesting it also needs updating post-rebrand.Consider renaming
IAMException→ThunderIDExceptionfor consistency (cross-file, so plan for a broader follow-up), or update the guideline text to reflect the current naming ifIAMExceptionis intentionally being kept.🤖 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 `@src/main/kotlin/dev/thunderid/android/ThunderIDError.kt` around lines 57 - 68, The rebrand is incomplete because `IAMException` still exposes the old naming alongside `ThunderIDErrorCode`, leaving a mixed public API. Decide whether to rename `IAMException` to `ThunderIDException` and update its constructor/usages in the SDK, or keep the class name and update the coding guideline text to match the new ThunderID naming. Make sure the change is applied consistently wherever the exception is referenced so `HttpClient`, `ThunderIDClient`, and related error handling all use the same naming scheme.Source: Coding guidelines
src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt (1)
107-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider
labelinstead ofplaceholder, and style the error text.Using
placeholdermeans the field name disappears once the user starts typing, which is a known usability pitfall for forms;label(which floats above the input) is the more idiomatic M3 pattern. Also,signInState.error?.let { Text(it) }renders errors with default text styling — considercolor = MaterialTheme.colorScheme.errorso users can visually distinguish it from other content.💡 Proposed fix
- signInState.error?.let { Text(it) } + signInState.error?.let { Text(it, color = MaterialTheme.colorScheme.error) } signInState.inputs.forEach { input -> val isPassword = input.type == "PASSWORD_INPUT" OutlinedTextField( value = signInState.fieldValue(input.name), onValueChange = { signInState.setField(input.name, it) }, modifier = Modifier.fillMaxWidth().semantics { contentDescription = input.name }, - placeholder = { Text(input.name) }, + label = { Text(input.name) },🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt` around lines 107 - 120, The SignIn form uses placeholder text where Material 3 prefers a label, so update the OutlinedTextField in SignIn to use label instead of placeholder for the input name. Also style the error rendering in signInState.error?.let so it uses MaterialTheme.colorScheme.error, keeping the error text visually distinct from normal content.
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/AuthScreen.kt`:
- Around line 275-304: The RecoverSheetContent composable is only displaying
static copy and a back button, so add a real password-recovery flow by
introducing an email TextField and a submit action inside RecoverSheetContent.
Wire the new submit handler to the appropriate recovery API or callback used by
AuthScreen, and keep onBackToSignIn as the cancel/back action so the sheet is
functional rather than informational only.
- Around line 80-210: Remove the trailing commas from all call-site argument
lists in AuthScreen and related composable calls to satisfy the ktlint rule.
Update the modifier chains, ButtonDefaults calls, ModalBottomSheet sheet content
invocations, and the DrawScope cubicTo calls so the last argument is not
followed by a comma; keep trailing commas only where the project allows them in
declarations. Use the existing symbols like AuthScreen, LoginSheetContent,
SignUpSheetContent, RecoverSheetContent, and the cubicTo drawing helpers to find
and clean all affected call sites.
In `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt`:
- Around line 117-379: Update the composable call sites in HomeTab, StatColumn,
StepRow, and ActionRow to remove trailing commas from the last argument,
matching the project style used in AuthScreen. Also wrap the two over-length
statements: the long Text call inside the Home screen content and the long
return expression in userDisplayName so every line stays within 120 characters.
- Around line 532-536: The TokenDebugScreen token fetch in LaunchedEffect(Unit)
is swallowing exceptions, which leaves token empty and the UI stuck in the
loading state. Update the HomeScreen flow so the catch branch stores a loadError
(or similar state) when thunder.client.getAccessToken() fails, and then have the
UI render that error instead of always showing “Loading…” when token.isEmpty().
Keep the fix localized to the token-loading state and the related render logic
so there is a visible failure path and retry-friendly behavior.
In `@src/main/kotlin/dev/thunderid/android/auth/FlowExecutionClient.kt`:
- Around line 29-40: The mapOf call inside FlowExecutionClient.initiate has a
trailing comma on the last argument, which violates the call-site formatting
rule. Remove the trailing comma after the "verbose" to true entry in the body
map construction, keeping the rest of initiate and the
httpClient.post("/flow/execute", ...) call unchanged.
In `@src/main/kotlin/dev/thunderid/android/http/HttpClient.kt`:
- Around line 70-98: In HttpClient’s request flow, the HttpURLConnection is
created and used without any connect/read timeouts and is never explicitly
disconnected. Add a shared timeout constant in HttpClient (for example in a
companion object) and apply it to the connection before sending the request,
then ensure the connection is always released by calling disconnect in a finally
block around the request/response handling in the same request method.
- Around line 48-64: The inline request path is capturing private members and a
private helper, which Kotlin disallows for inline functions. Keep get() and
post() inline, but make request() non-inline and route the shared logic through
a normal helper that accepts the type token, or alternatively expose the touched
members/helper with `@PublishedApi`. Use the existing HttpClient.request, get(),
and post() symbols to update the implementation without relying on private state
inside the inline body.
In `@src/main/kotlin/dev/thunderid/android/StorageAdapter.kt`:
- Around line 44-58: Remove the trailing comma from the last argument in the
EncryptedSharedPreferences.create call inside EncryptedStorageAdapter’s prefs
initialization. Keep the rest of the argument list unchanged, and ensure the
call-site invocation follows the no-trailing-comma guideline while leaving the
constructor and parameter declarations elsewhere in StorageAdapter.kt as-is.
In `@src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt`:
- Around line 133-136: The callback handling in ThunderIDClient should not
extract the authorization code with substring matching, because
substringAfter("code=") can return the full URL when the parameter is missing.
Update the code that computes code to parse the callback URL query parameters
properly in ThunderIDClient, using the existing callback URL value and then
reading the code parameter by name. Keep the existing IAMException INVALID_GRANT
behavior for missing or empty code values, but ensure it is triggered only after
actual query parsing rather than string searching.
- Around line 113-124: The authorization URL in buildSignInUrl is built by
manual string concatenation, so query values like redirect_uri, scope, and any
optional SignInOptions parameters can remain unescaped. Update this flow to
URL-encode each parameter before appending it to the authorize endpoint, and
apply the same encoding to any future values added to the params StringBuilder
so reserved characters and spaces are preserved correctly.
- Around line 75-87: The reInitialize() flow in ThunderIDClient is dropping the
resolved storage adapter by always passing null into initialize(), which breaks
clients that were originally started with a storage-backed config. Update
reInitialize() so it preserves and reuses the existing storage adapter from the
current initialized state (using the same resolved storage reference that
initialize(config, storage) set up) when rebuilding the updated ThunderIDConfig,
and keep the logic localized to ThunderIDClient.reInitialize and initialize.
In `@src/main/kotlin/dev/thunderid/android/token/TokenRefresher.kt`:
- Around line 48-62: The TokenRefresher.refresh method builds a call-site
mapOf(...) request body with a trailing comma on the last entry, which violates
the ktlint call-site comma rule. Remove the trailing comma from the final entry
in the body map inside refresh so the OAuth token refresh request still uses the
same keys and values but passes ktlintCheck.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt`:
- Around line 177-181: The debug log in SignIn’s response handling is leaking
the full FlowStepData payload via response.data. Update the android.util.Log.d
call in SignIn to log only non-sensitive summary information such as
response.flowStatus and the sizes of response.data?.inputs and
response.data?.actions, and remove the data=${response.data} portion entirely.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt`:
- Around line 181-186: `SignUp` currently leaves `FlowStatus.INCOMPLETE` as a
no-op, so the state never refreshes for multi-step registration. Update the
`when` branch in `SignUp.kt` near the `FlowStatus.COMPLETE`/`PROMPT_ONLY`
handling to treat `INCOMPLETE` the same as the other intermediate states by
calling `state.update(response)` (matching the `SignIn` flow behavior), while
keeping `COMPLETE` as the completion path.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/user/UserDropdown.kt`:
- Around line 72-76: The UserDropdown sign-out click handler is invoking
onSignOutComplete immediately after signOut(), but signOut() only starts the
logout coroutine. Update the sign-out flow in UserDropdown so the completion
callback runs only after the client sign-out and refresh logic has finished,
likely by moving the callback into the end of the logout completion path in
signOut() or by making the click handler await the logout result before calling
onSignOutComplete.
In `@src/test/kotlin/dev/thunderid/android/ThunderIDClientTest.kt`:
- Around line 80-93: The ThunderIDConfig call in ThunderIDClientTest uses an
invalid trailing comma on the last constructor argument at a call site. Update
the `getConfiguration returns config after init` test so the
`ThunderIDConfig(...)` invocation no longer ends its final `scopes` argument
with a comma, while keeping the rest of the setup and assertions unchanged.
---
Outside diff comments:
In `@samples/quickstart/build.gradle.kts`:
- Around line 24-34: The values returned by config() are being interpolated
directly into buildConfigField as raw source, so unescaped quotes or backslashes
can break the generated BuildConfig or inject invalid code. Update the
quickstart build script around config() and the THUNDERID_* buildConfigField
calls to escape these values before wrapping them in Kotlin string literals, or
use a safe helper that produces properly escaped source for BuildConfig fields.
In `@src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt`:
- Around line 206-216: Normalize all JWT payload parsing failures in
decodeJwtToken to IAMException by catching Base64/JSON parsing exceptions around
the payload decode and JSONObject creation. Keep the existing INVALID_INPUT path
for bad token shape, and map malformed payload content to
ThunderIDErrorCode.INVALID_INPUT with a clear message instead of letting
platform exceptions escape. Use the decodeJwtToken function as the single entry
point for this typed error handling.
In `@src/main/kotlin/dev/thunderid/compose/components/guards/SignedIn.kt`:
- Around line 30-31: The SignedIn guard currently falls back whenever isSignedIn
is false, which causes a brief signed-out state during ThunderIDProvider
initialization. Update SignedIn to also check the initialization/loading state
from LocalThunderID (for example, using isInitialized or isLoading) before
calling fallback(), or otherwise make the intended Loading-wrapper requirement
explicit in the guard’s behavior using the SignedIn symbol.
In `@src/main/kotlin/dev/thunderid/compose/components/presentation/user/User.kt`:
- Around line 30-38: Update UserObject so its identity fallback includes email
before using the anonymous i18n string. In the UserObject composable, adjust the
label selection order to use user.displayName, then user.username, then
user.email, and only then i18n.resolve("user.anonymous"), matching the fallback
behavior used in UserDropdown and the user model.
- Around line 41-48: The BaseUserObject composable ignores its modifier
parameter, so callers cannot control layout or semantics. Update BaseUserObject
to wrap the content lambda in a container that accepts and forwards the passed
modifier, using the BaseUserObject symbol as the place to apply it while still
passing state.user into content.
In `@src/test/kotlin/dev/thunderid/android/ThunderIDClientTest.kt`:
- Around line 121-136: The TokenResponse constructor call in ThunderIDClientTest
uses a trailing comma on the last call-site argument, which violates the coding
guideline. Update the TokenResponse invocation in TokenStore saves and retrieves
tokens so the final argument is written without a trailing comma, keeping the
rest of the test unchanged.
---
Minor comments:
In `@AGENTS.md`:
- Around line 47-67: The fenced directory-tree block in AGENTS.md is unlabeled,
which triggers markdownlint. Update that existing markdown fence to use a text
label so it renders correctly and passes linting, keeping the directory tree
content unchanged.
In `@samples/quickstart/src/main/AndroidManifest.xml`:
- Around line 7-8: The <application> section in AndroidManifest is missing the
network security config reference, so the debug trust anchors from
network_security_config.xml are never applied. Update the <application>
attributes in AndroidManifest to include android:networkSecurityConfig pointing
at the existing network_security_config.xml resource, keeping the change scoped
to the quickstart app’s manifest.
In `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt`:
- Around line 720-730: The caught exception in decodeJwtPayload is intentionally
unused, so it triggers detekt’s SwallowedException rule. Rename the caught
Exception parameter to the allowed underscore convention in decodeJwtPayload and
keep the empty fallback behavior unchanged; this satisfies the default
allowedExceptionNameRegex without adding logging or altering the return path.
- Around line 70-75: The import block is out of alphabetical order in
HomeScreen, with java.* imports appearing after kotlinx.* imports. Reorder the
imports so java.text.SimpleDateFormat and java.util.Date/Locale come before
kotlinx.coroutines.Dispatchers, delay, and withContext, and keep the list
alphabetized consistently.
In `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/MainActivity.kt`:
- Around line 44-45: Remove the trailing comma at the ThunderIDConfig call site
in MainActivity, since ktlint forbids trailing commas on the last argument in
call expressions. Update the argument list around the ThunderIDConfig(...)
invocation so allowInsecureConnections is the final argument without a comma,
keeping the rest of the call unchanged.
In `@src/main/kotlin/dev/thunderid/android/http/HttpClient.kt`:
- Around line 21-34: The Kotlin file still has ktlint issues: the import block
in HttpClient must be alphabetized, and the long statement in the HttpClient
logic around the line that currently exceeds 120 characters needs to be wrapped.
Update the relevant imports in HttpClient.kt so they are sorted consistently,
and refactor the long expression in the affected method to fit within the
line-length limit while preserving behavior.
In `@src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt`:
- Around line 52-57: The remaining ktlint issues in ThunderIDClient should be
cleaned up by removing trailing commas from the affected call sites and
reflowing any overlong expressions so no Kotlin line exceeds 120 characters.
Update the relevant sections in ThunderIDClient and the related call sites
around the referenced symbols/blocks to match ktlint formatting rules, then
rerun ktlintCheck to confirm the file is fully clean.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt`:
- Around line 110-120: Trailing commas are present on the last argument of
several call sites and violate ktlint rules. Remove the trailing comma from the
final parameter in the `OutlinedTextField`, `Button`, and
`EmbeddedSignInPayload` invocations in `SignIn.kt`, keeping trailing commas only
for multi-line declarations, not function calls.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt`:
- Around line 104-110: Remove the trailing commas from the last arguments at
call sites to match ktlint policy. Update the `BasicTextField` call in
`SignUp.kt` so the `modifier` argument no longer ends with a comma, and do the
same for the `EmbeddedSignInPayload` construction so `challengeToken =
signUpState.challengeToken` is the final argument without a trailing comma.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/user/UserProfile.kt`:
- Around line 99-114: The load failure handling in UserProfile’s profile-fetch
path can store a null error because it assigns e.message directly. Update the
catch block around thunderState.client.getUserProfile() to always set a non-null
fallback message when the exception has no message, so the error state reliably
triggers instead of falling through to the edit form.
In `@src/main/kotlin/dev/thunderid/compose/i18n/DefaultStrings.kt`:
- Around line 23-57: The mapOf call in DefaultStrings.all uses a trailing comma
on its last entry, which violates the call-site formatting rule. Remove the
trailing comma after the final inviteUser.loading entry while keeping the rest
of the string map unchanged. Use the DefaultStrings.all property and the mapOf
builder as the place to make this cleanup.
In `@src/test/kotlin/dev/thunderid/compose/ComponentTests.kt`:
- Around line 50-58: Remove the trailing commas from the call-site argument
lists in ComponentTests, including the ThunderIDI18n constructor calls and the
listOf usage referenced by the review. Keep the declarations unchanged, but
update these invocation sites so the last argument in each call no longer ends
with a comma while preserving the existing test behavior.
---
Nitpick comments:
In `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt`:
- Around line 404-413: The back-navigation row in HomeScreen uses plain Text
inside a clickable Row, so it lacks an accessible announcement for screen
readers. Update the back row(s) built with the clickable Row/“‹ Home” text to
include semantics with a meaningful content description or role so assistive
tech announces it as a back action; apply the same fix to the other matching
back row referenced in the comment.
- Around line 542-562: The claim extraction logic in HomeScreen is manually
parsing JSON with regex and a hand-rolled formatter, which is brittle and
duplicates work. Replace the raw string parsing in prettyPrintJson and the
expiryText/issuer/scopes remember blocks with a proper JSON parse using
org.json.JSONObject, then read exp, iss, and scope from the parsed object
instead of re-extracting from rawJson. Keep the existing UI values and fallback
behavior, but centralize the JSON handling so escaped quotes and other valid
JSON edge cases are handled correctly.
- Around line 1-813: The HomeScreen file is far over the new-file size guideline
because it combines HomeScreen, HomeTab, ProfileScreen, TokenDebugScreen, and
all shared helpers in one place. Split the UI into separate composables/files by
feature: move ProfileScreen-related code into a ProfileScreen file,
TokenDebugScreen into its own file, and extract shared helpers like StatColumn,
StepRow, ActionRow, SectionHeader, DetailCard, DetailRow, and the JWT/date/user
utility functions into a shared UI/helpers file. Keep HomeScreen focused on
navigation and wiring, and update imports/usages so the top-level file stays
well under the ~400-line limit.
In `@settings.gradle.kts`:
- Line 9: The root project name is too generic and makes the multi-module repo
harder to पहचान in IDEs, build logs, and CI artifacts. Update the
rootProject.name assignment in settings.gradle.kts back to a more specific
project name like the original repository name, keeping the change localized to
the root project identity so all modules inherit a clearer label.
In `@src/main/kotlin/dev/thunderid/android/ThunderIDConfig.kt`:
- Around line 46-49: The allowInsecureConnections flag in ThunderIDConfig
currently relies only on a comment, but it is used by ThunderIDClient/HttpClient
to bypass TLS verification. Add a runtime safeguard where the flag is consumed,
such as checking BuildConfig.DEBUG before enabling the trust-all path or logging
a strong warning and refusing the insecure mode in non-debug builds. Keep the
guard close to the HttpClient setup so a true value cannot silently disable
certificate validation in release configurations.
In `@src/main/kotlin/dev/thunderid/android/ThunderIDError.kt`:
- Around line 57-68: The rebrand is incomplete because `IAMException` still
exposes the old naming alongside `ThunderIDErrorCode`, leaving a mixed public
API. Decide whether to rename `IAMException` to `ThunderIDException` and update
its constructor/usages in the SDK, or keep the class name and update the coding
guideline text to match the new ThunderID naming. Make sure the change is
applied consistently wherever the exception is referenced so `HttpClient`,
`ThunderIDClient`, and related error handling all use the same naming scheme.
In
`@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt`:
- Around line 107-120: The SignIn form uses placeholder text where Material 3
prefers a label, so update the OutlinedTextField in SignIn to use label instead
of placeholder for the input name. Also style the error rendering in
signInState.error?.let so it uses MaterialTheme.colorScheme.error, keeping the
error text visually distinct from normal content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+80
to
+210
| Box( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(BgLight), | ||
| ) { | ||
| Column( | ||
| modifier = Modifier.fillMaxSize(), | ||
| verticalArrangement = Arrangement.SpaceBetween, | ||
| ) { | ||
| // Upper area — scrollable | ||
| Column( | ||
| modifier = Modifier | ||
| .weight(1f) | ||
| .verticalScroll(rememberScrollState()) | ||
| .padding(horizontal = 32.dp), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| Spacer(Modifier.height(72.dp)) | ||
|
|
||
| // Logo mark | ||
| ThunderLogoMark( | ||
| modifier = Modifier | ||
| .height(80.dp) | ||
| .width(65.dp), | ||
| darkColor = TextPrimary, | ||
| blueColor = PrimaryBlue, | ||
| ) | ||
|
|
||
| Spacer(Modifier.height(32.dp)) | ||
|
|
||
| // Headline | ||
| Text( | ||
| text = "Authentication\nfor developers.", | ||
| fontSize = 28.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| letterSpacing = (-0.5).sp, | ||
| color = TextPrimary, | ||
| textAlign = TextAlign.Center, | ||
| lineHeight = 34.sp, | ||
| ) | ||
|
|
||
| Spacer(Modifier.height(12.dp)) | ||
|
|
||
| // Subtext | ||
| Text( | ||
| text = "OAuth 2.0, PKCE, MFA, and JWT —\nout of the box in minutes.", | ||
| fontSize = 15.sp, | ||
| color = TextMuted, | ||
| textAlign = TextAlign.Center, | ||
| lineHeight = 22.sp, | ||
| ) | ||
|
|
||
| Spacer(Modifier.height(28.dp)) | ||
|
|
||
| // Feature tags | ||
| Row( | ||
| modifier = Modifier.horizontalScroll(rememberScrollState()), | ||
| horizontalArrangement = Arrangement.spacedBy(8.dp), | ||
| ) { | ||
| featureTags.forEach { tag -> | ||
| Text( | ||
| text = tag, | ||
| fontSize = 12.sp, | ||
| fontWeight = FontWeight.Medium, | ||
| color = PrimaryBlue, | ||
| modifier = Modifier | ||
| .border(1.dp, BorderLight, RoundedCornerShape(20.dp)) | ||
| .padding(horizontal = 12.dp, vertical = 6.dp), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| Spacer(Modifier.height(48.dp)) | ||
| } | ||
|
|
||
| // Bottom CTAs | ||
| Column( | ||
| modifier = Modifier | ||
| .padding(horizontal = 24.dp) | ||
| .padding(bottom = 40.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| ) { | ||
| Button( | ||
| onClick = { showSheet = "signup" }, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .height(50.dp), | ||
| shape = RoundedCornerShape(10.dp), | ||
| colors = ButtonDefaults.buttonColors(containerColor = PrimaryBlue), | ||
| ) { | ||
| Text("Get started", fontSize = 16.sp, fontWeight = FontWeight.SemiBold) | ||
| } | ||
|
|
||
| OutlinedButton( | ||
| onClick = { showSheet = "login" }, | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .height(50.dp), | ||
| shape = RoundedCornerShape(10.dp), | ||
| border = androidx.compose.foundation.BorderStroke(1.5.dp, PrimaryBlue), | ||
| colors = ButtonDefaults.outlinedButtonColors(contentColor = PrimaryBlue), | ||
| ) { | ||
| Text("Sign in", fontSize = 16.sp, fontWeight = FontWeight.SemiBold) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Bottom sheets | ||
| if (showSheet != null) { | ||
| val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) | ||
| ModalBottomSheet( | ||
| onDismissRequest = { showSheet = null }, | ||
| sheetState = sheetState, | ||
| ) { | ||
| when (showSheet) { | ||
| "login" -> LoginSheetContent( | ||
| applicationId = applicationId, | ||
| onForgotPassword = { showSheet = "recover" }, | ||
| onSignUp = { showSheet = "signup" }, | ||
| ) | ||
| "signup" -> SignUpSheetContent( | ||
| onSignIn = { showSheet = "login" }, | ||
| ) | ||
| "recover" -> RecoverSheetContent( | ||
| onBackToSignIn = { showSheet = "login" }, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trailing commas on call-site arguments violate the project's ktlint rule.
Per the repo's coding guidelines, call sites must not have a trailing comma on the last argument (only declarations should). This file has it pervasively, e.g. .background(BgLight), (line 84), contentAlignment in Box calls, colors = ButtonDefaults.buttonColors(...), (line 168), the sheet-content invocations (lines 199, 203, 206), and the cubicTo(...) calls (e.g. line 353, 372, 377). This will fail ktlintCheck.
🎨 Example fix (pattern repeats throughout file)
Box(
modifier = Modifier
.fillMaxSize()
- .background(BgLight),
+ .background(BgLight)
) {As per coding guidelines, "Do not use a trailing comma on the last argument at call sites."
Also applies to: 344-378
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/AuthScreen.kt`
around lines 80 - 210, Remove the trailing commas from all call-site argument
lists in AuthScreen and related composable calls to satisfy the ktlint rule.
Update the modifier chains, ButtonDefaults calls, ModalBottomSheet sheet content
invocations, and the DrawScope cubicTo calls so the last argument is not
followed by a comma; keep trailing commas only where the project allows them in
declarations. Use the existing symbols like AuthScreen, LoginSheetContent,
SignUpSheetContent, RecoverSheetContent, and the cubicTo drawing helpers to find
and clean all affected call sites.
Source: Coding guidelines
Comment on lines
+275
to
+304
| @Composable | ||
| private fun RecoverSheetContent(onBackToSignIn: () -> Unit) { | ||
| Column( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(horizontal = 24.dp) | ||
| .padding(bottom = 48.dp), | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| ) { | ||
| SheetTitle("Reset password") | ||
| Spacer(Modifier.height(8.dp)) | ||
| Text( | ||
| text = "Enter your email below", | ||
| fontSize = 14.sp, | ||
| color = TextMuted, | ||
| modifier = Modifier.padding(start = 0.dp), | ||
| textAlign = TextAlign.Start, | ||
| ) | ||
| Spacer(Modifier.height(32.dp)) | ||
| OutlinedButton( | ||
| onClick = onBackToSignIn, | ||
| modifier = Modifier.fillMaxWidth().height(48.dp), | ||
| shape = RoundedCornerShape(10.dp), | ||
| border = androidx.compose.foundation.BorderStroke(1.dp, PrimaryBlue), | ||
| colors = ButtonDefaults.outlinedButtonColors(contentColor = PrimaryBlue), | ||
| ) { | ||
| Text("Back to sign in", fontWeight = FontWeight.Medium) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Recover-password sheet has no way to actually recover a password.
RecoverSheetContent only shows static text ("Enter your email below") and a "Back to sign in" button — there's no email TextField or submit action. As a quickstart sample meant to demonstrate real auth flows, this is misleading: a developer copying this pattern gets a non-functional recovery screen.
Want me to draft the missing email field + submit handler (wired to the SDK's recovery API, if available) for this composable?
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/AuthScreen.kt`
around lines 275 - 304, The RecoverSheetContent composable is only displaying
static copy and a back button, so add a real password-recovery flow by
introducing an email TextField and a submit action inside RecoverSheetContent.
Wire the new submit handler to the appropriate recovery API or callback used by
AuthScreen, and keep onBackToSignIn as the cancel/back action so the sheet is
functional rather than informational only.
Comment on lines
117
to
379
| @Composable | ||
| private fun ExploreTab(onProfileTap: () -> Unit) { | ||
| private fun HomeTab(onNavigate: (String) -> Unit) { | ||
| val thunder = LocalThunderID.current | ||
| val cs = MaterialTheme.colorScheme | ||
| var categoryIndex by remember { mutableIntStateOf(0) } | ||
| var sortIndex by remember { mutableIntStateOf(0) } | ||
|
|
||
| val firstName = remember(thunder.user) { | ||
| (thunder.user?.claims?.get("given_name") as? String)?.takeIf { it.isNotEmpty() } | ||
| ?: thunder.user?.displayName?.split(" ")?.firstOrNull() | ||
| ?: "there" | ||
|
|
||
| val displayName = remember(thunder.user) { userDisplayName(thunder.user) } | ||
| val initials = remember(displayName) { userInitials(displayName) } | ||
| val email = remember(thunder.user) { thunder.user?.email ?: "" } | ||
|
|
||
| val dateLabel = remember { | ||
| SimpleDateFormat("EEEE, MMMM d", Locale.ENGLISH).format(Date()).uppercase() | ||
| } | ||
| val greeting = remember { | ||
| val hour = java.util.Calendar.getInstance().get(java.util.Calendar.HOUR_OF_DAY) | ||
| when { | ||
| hour < 12 -> "Good morning." | ||
| hour < 17 -> "Good afternoon." | ||
| else -> "Good evening." | ||
| } | ||
| } | ||
|
|
||
| val claims = thunder.user?.claims | ||
| val authTime = remember(claims) { claimAsEpochSeconds(claims, "auth_time") } | ||
| val exp = remember(claims) { claimAsEpochSeconds(claims, "exp") } | ||
| val organisationName = remember(thunder.user) { | ||
| runCatching { thunder.client.getConfiguration().organizationHandle } | ||
| .getOrNull() | ||
| ?.takeIf { it.isNotBlank() } | ||
| ?: "Default" | ||
| } | ||
|
|
||
| var nowSeconds by remember { mutableStateOf(System.currentTimeMillis() / 1000) } | ||
| LaunchedEffect(exp) { | ||
| while (exp != null) { | ||
| nowSeconds = System.currentTimeMillis() / 1000 | ||
| delay(1000) | ||
| } | ||
| } | ||
|
|
||
| val signedInAtLabel = remember(authTime) { formatSignedInAt(authTime) } | ||
| val expiresInLabel = remember(exp, nowSeconds) { formatExpiresIn(exp, nowSeconds) } | ||
|
|
||
| Column( | ||
| modifier = Modifier | ||
| .fillMaxSize() | ||
| .background(LightBg) | ||
| .verticalScroll(rememberScrollState()), | ||
| ) { | ||
| // Top bar | ||
| Row( | ||
| modifier = Modifier.fillMaxWidth().padding(start = 24.dp, end = 16.dp, top = 20.dp), | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| Spacer(Modifier.height(56.dp)) | ||
|
|
||
| // User identity section | ||
| Column( | ||
| modifier = Modifier.padding(horizontal = 24.dp), | ||
| ) { | ||
| Icon(Icons.Filled.Home, contentDescription = null, tint = cs.primary) | ||
| Spacer(Modifier.width(6.dp)) | ||
| Text("ACME Booking", color = cs.primary, fontWeight = FontWeight.Bold, fontSize = 17.sp) | ||
| Spacer(Modifier.weight(1f)) | ||
| UserAvatar(user = thunder.user, radius = 18.dp, modifier = Modifier.clickable { onProfileTap() }) | ||
| Spacer(Modifier.width(8.dp)) | ||
| Row(verticalAlignment = Alignment.CenterVertically) { | ||
| // Avatar | ||
| Box( | ||
| modifier = Modifier | ||
| .size(52.dp) | ||
| .background(PrimaryBlue, CircleShape), | ||
| contentAlignment = Alignment.Center, | ||
| ) { | ||
| Text( | ||
| text = initials, | ||
| color = Color.White, | ||
| fontSize = 18.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| ) | ||
| } | ||
| Spacer(Modifier.width(14.dp)) | ||
| Column { | ||
| Text( | ||
| text = displayName, | ||
| fontSize = 18.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = TextPrimary, | ||
| ) | ||
| Spacer(Modifier.height(2.dp)) | ||
| Text( | ||
| text = email, | ||
| fontSize = 13.sp, | ||
| color = TextMuted, | ||
| ) | ||
| Spacer(Modifier.height(6.dp)) | ||
| // Session active badge | ||
| Row(verticalAlignment = Alignment.CenterVertically) { | ||
| Box( | ||
| modifier = Modifier | ||
| .size(7.dp) | ||
| .background(SuccessGreen, CircleShape), | ||
| ) | ||
| Spacer(Modifier.width(5.dp)) | ||
| Text( | ||
| text = "Session active", | ||
| fontSize = 12.sp, | ||
| color = SuccessGreen, | ||
| fontWeight = FontWeight.Medium, | ||
| ) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Welcome heading | ||
| Text( | ||
| text = "Where Would you\nLike to Stay, $firstName?", | ||
| style = MaterialTheme.typography.headlineSmall.copy(fontWeight = FontWeight.Bold), | ||
| modifier = Modifier.padding(start = 24.dp, end = 24.dp, top = 24.dp), | ||
| ) | ||
| Spacer(Modifier.height(32.dp)) | ||
|
|
||
| // Search bar | ||
| // Date + greeting | ||
| Column(modifier = Modifier.padding(horizontal = 24.dp)) { | ||
| Text( | ||
| text = dateLabel, | ||
| fontSize = 12.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = TextMuted, | ||
| letterSpacing = 1.sp, | ||
| ) | ||
| Spacer(Modifier.height(4.dp)) | ||
| Text( | ||
| text = greeting, | ||
| fontSize = 22.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = TextPrimary, | ||
| ) | ||
| } | ||
|
|
||
| Spacer(Modifier.height(24.dp)) | ||
|
|
||
| // Stats row | ||
| Row( | ||
| modifier = Modifier | ||
| .padding(horizontal = 24.dp, vertical = 16.dp) | ||
| .padding(horizontal = 24.dp) | ||
| .fillMaxWidth() | ||
| .background(cs.surfaceContainerHighest, CircleShape) | ||
| .padding(horizontal = 16.dp, vertical = 12.dp), | ||
| .border(1.dp, BorderLight, RoundedCornerShape(12.dp)) | ||
| .padding(vertical = 18.dp), | ||
| horizontalArrangement = Arrangement.SpaceEvenly, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| ) { | ||
| Icon(Icons.Filled.Search, contentDescription = null, tint = cs.onSurfaceVariant) | ||
| Spacer(Modifier.width(8.dp)) | ||
| Text("Search destinations...", color = cs.onSurfaceVariant) | ||
| StatColumn(value = signedInAtLabel, label = "SIGNED IN AT") | ||
| VerticalDivider(modifier = Modifier.height(36.dp), color = BorderLight) | ||
| StatColumn(value = expiresInLabel, label = "EXPIRES IN") | ||
| VerticalDivider(modifier = Modifier.height(36.dp), color = BorderLight) | ||
| StatColumn(value = organisationName, label = "ORGANISATION") | ||
| } | ||
|
|
||
| // Category chips | ||
| Row( | ||
| modifier = Modifier | ||
| .horizontalScroll(rememberScrollState()) | ||
| .padding(start = 24.dp, end = 12.dp, bottom = 4.dp), | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp), | ||
| ) { | ||
| categories.forEachIndexed { i, label -> | ||
| val selected = i == categoryIndex | ||
| Column( | ||
| horizontalAlignment = Alignment.CenterHorizontally, | ||
| modifier = Modifier.clickable { categoryIndex = i }, | ||
| ) { | ||
| Box( | ||
| modifier = Modifier | ||
| .size(56.dp) | ||
| .background( | ||
| if (selected) cs.primary else cs.surfaceContainerHighest, | ||
| RoundedCornerShape(14.dp), | ||
| ), | ||
| contentAlignment = Alignment.Center, | ||
| ) { | ||
| Icon( | ||
| imageVector = categoryIcons[i], | ||
| contentDescription = label, | ||
| tint = if (selected) cs.onPrimary else cs.onSurfaceVariant, | ||
| modifier = Modifier.size(24.dp), | ||
| ) | ||
| } | ||
| Spacer(Modifier.height(6.dp)) | ||
| Text( | ||
| text = label, | ||
| fontSize = 11.sp, | ||
| fontWeight = if (selected) FontWeight.Bold else FontWeight.Normal, | ||
| color = if (selected) cs.primary else cs.onSurfaceVariant, | ||
| ) | ||
| } | ||
| } | ||
| Spacer(Modifier.height(28.dp)) | ||
|
|
||
| // "What's next" section | ||
| Text( | ||
| text = "WHAT'S NEXT", | ||
| fontSize = 10.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = TextMuted, | ||
| letterSpacing = 1.5.sp, | ||
| modifier = Modifier.padding(horizontal = 24.dp), | ||
| ) | ||
|
|
||
| Spacer(Modifier.height(8.dp)) | ||
|
|
||
| val steps = listOf( | ||
| Triple("01", "Secure your API", "Add token validation to your backend."), | ||
| Triple("02", "Add social login", "GitHub, Google, and OIDC providers."), | ||
| Triple("03", "Enable MFA", "TOTP and passkey support."), | ||
| Triple("04", "Explore the SDK", "API reference and guides."), | ||
| ) | ||
|
|
||
| steps.forEach { (num, title, subtitle) -> | ||
| StepRow(number = num, title = title, subtitle = subtitle) | ||
| } | ||
|
|
||
| // Sort tabs | ||
| Row( | ||
| Spacer(Modifier.height(20.dp)) | ||
|
|
||
| // Action rows | ||
| ActionRow(label = "My profile", onClick = { onNavigate("profile") }) | ||
| ActionRow(label = "Token debug", onClick = { onNavigate("token") }) | ||
| ActionRow(label = "Settings", onClick = {}) | ||
|
|
||
| // Sign out | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .padding(horizontal = 24.dp, vertical = 20.dp), | ||
| verticalAlignment = Alignment.Bottom, | ||
| .border(width = 1.dp, color = BorderLight, shape = RoundedCornerShape(0.dp)) | ||
| .padding(horizontal = 24.dp, vertical = 4.dp), | ||
| ) { | ||
| sorts.forEachIndexed { i, label -> | ||
| val selected = i == sortIndex | ||
| Column( | ||
| modifier = Modifier | ||
| .clickable { sortIndex = i } | ||
| .padding(end = if (i < sorts.size - 1) 20.dp else 0.dp), | ||
| ) { | ||
| Text( | ||
| text = label, | ||
| fontSize = 15.sp, | ||
| fontWeight = if (selected) FontWeight.Bold else FontWeight.Normal, | ||
| color = if (selected) cs.onSurface else cs.onSurfaceVariant, | ||
| ) | ||
| Spacer(Modifier.height(3.dp)) | ||
| if (selected) { | ||
| Box( | ||
| modifier = Modifier | ||
| .width(28.dp) | ||
| .height(2.dp) | ||
| .background(cs.primary, RoundedCornerShape(1.dp)), | ||
| ) | ||
| } else { | ||
| Spacer(Modifier.height(2.dp)) | ||
| } | ||
| } | ||
| } | ||
| Spacer(Modifier.weight(1f)) | ||
| Text("See More", color = cs.primary, fontWeight = FontWeight.SemiBold, fontSize = 14.sp) | ||
| SignOutButton(modifier = Modifier.fillMaxWidth()) | ||
| } | ||
|
|
||
| // Listings grid (2 columns) | ||
| Column( | ||
| modifier = Modifier.padding(horizontal = 24.dp), | ||
| verticalArrangement = Arrangement.spacedBy(12.dp), | ||
| Spacer(Modifier.height(40.dp)) | ||
| } | ||
| } | ||
|
|
||
| @Composable | ||
| private fun StatColumn(value: String, label: String) { | ||
| Column(horizontalAlignment = Alignment.CenterHorizontally) { | ||
| Text( | ||
| text = value, | ||
| fontSize = 20.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = TextPrimary, | ||
| ) | ||
| Spacer(Modifier.height(2.dp)) | ||
| Text( | ||
| text = label, | ||
| fontSize = 10.sp, | ||
| fontWeight = FontWeight.Bold, | ||
| color = TextMuted, | ||
| letterSpacing = 0.8.sp, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| @Composable | ||
| private fun StepRow(number: String, title: String, subtitle: String) { | ||
| Row( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .border(width = 0.dp, color = Color.Transparent) | ||
| .padding(horizontal = 24.dp, vertical = 14.dp), | ||
| verticalAlignment = Alignment.Top, | ||
| ) { | ||
| Text( | ||
| text = number, | ||
| fontSize = 9.sp, | ||
| fontFamily = FontFamily.Monospace, | ||
| color = PrimaryBlue, | ||
| fontWeight = FontWeight.Bold, | ||
| modifier = Modifier.padding(top = 2.dp), | ||
| ) | ||
| Spacer(Modifier.width(14.dp)) | ||
| Column { | ||
| Text(text = title, fontSize = 14.sp, fontWeight = FontWeight.SemiBold, color = TextPrimary) | ||
| Spacer(Modifier.height(2.dp)) | ||
| Text(text = subtitle, fontSize = 13.sp, color = TextMuted) | ||
| } | ||
| } | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .height(1.dp) | ||
| .background(BorderLight), | ||
| ) | ||
| } | ||
|
|
||
| @Composable | ||
| private fun ActionRow(label: String, onClick: () -> Unit) { | ||
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .border(width = 1.dp, color = BorderLight, shape = RoundedCornerShape(0.dp)) | ||
| .clickable(onClick = onClick) | ||
| .padding(horizontal = 24.dp, vertical = 16.dp), | ||
| ) { | ||
| Row( | ||
| modifier = Modifier.fillMaxWidth(), | ||
| horizontalArrangement = Arrangement.SpaceBetween, | ||
| verticalAlignment = Alignment.CenterVertically, | ||
| ) { | ||
| listings.chunked(2).forEach { row -> | ||
| Row(horizontalArrangement = Arrangement.spacedBy(12.dp)) { | ||
| row.forEach { listing -> | ||
| ListingCard(listing = listing, modifier = Modifier.weight(1f)) | ||
| } | ||
| if (row.size == 1) Spacer(Modifier.weight(1f)) | ||
| } | ||
| } | ||
| Text( | ||
| text = label, | ||
| fontSize = 15.sp, | ||
| color = TextPrimary, | ||
| fontWeight = FontWeight.Medium, | ||
| ) | ||
| Text(text = "›", fontSize = 20.sp, color = TextMuted) | ||
| } | ||
| Spacer(Modifier.height(24.dp)) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trailing commas on call-site arguments (pervasive), and two lines exceed the 120-char limit.
Same call-site trailing-comma pattern as in AuthScreen.kt recurs here (e.g. lines 163, 177, 249, 605-607, 619, 627). Additionally, two lines exceed the 120-character limit:
- Line 620: the
Text(...)call is ~132 chars. - Line 803: the
userDisplayNamereturn statement is ~135 chars.
📏 Wrap the two over-length lines
- Text(text = "ACCESS TOKEN", fontSize = 10.sp, fontWeight = FontWeight.Bold, color = TextMuted, letterSpacing = 1.sp)
+ Text(
+ text = "ACCESS TOKEN",
+ fontSize = 10.sp,
+ fontWeight = FontWeight.Bold,
+ color = TextMuted,
+ letterSpacing = 1.sp
+ )- return full.ifEmpty { user.displayName?.takeIf { it.isNotEmpty() } ?: user.username ?: user.email?.substringBefore("@") ?: "User" }
+ return full.ifEmpty {
+ user.displayName?.takeIf { it.isNotEmpty() }
+ ?: user.username
+ ?: user.email?.substringBefore("@")
+ ?: "User"
+ }As per coding guidelines, "Do not use a trailing comma on the last argument at call sites" and "Keep lines at 120 characters or fewer; wrap long function signatures and call sites."
Also applies to: 564-714
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt`
around lines 117 - 379, Update the composable call sites in HomeTab, StatColumn,
StepRow, and ActionRow to remove trailing commas from the last argument,
matching the project style used in AuthScreen. Also wrap the two over-length
statements: the long Text call inside the Home screen content and the long
return expression in userDisplayName so every line stays within 120 characters.
Source: Coding guidelines
Comment on lines
+532
to
536
| LaunchedEffect(Unit) { | ||
| try { | ||
| token = thunder.client.getAccessToken() | ||
| } catch (_: Exception) {} | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Silently swallowed error leaves TokenDebugScreen stuck on "Loading…" forever.
If thunder.client.getAccessToken() throws, the exception is discarded and token stays "". Since the UI shows "Loading…" whenever token.isEmpty(), the user gets no error indication and no retry path if the fetch fails.
🛠️ Surface the failure
+ var loadError by remember { mutableStateOf<String?>(null) }
LaunchedEffect(Unit) {
try {
token = thunder.client.getAccessToken()
- } catch (_: Exception) {}
+ } catch (e: Exception) {
+ loadError = e.message ?: "Failed to load token"
+ }
}Then render loadError instead of an indefinite "Loading…" state.
🤖 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 `@samples/quickstart/src/main/kotlin/dev/thunderid/quickstart/HomeScreen.kt`
around lines 532 - 536, The TokenDebugScreen token fetch in LaunchedEffect(Unit)
is swallowing exceptions, which leaves token empty and the UI stuck in the
loading state. Update the HomeScreen flow so the catch branch stores a loadError
(or similar state) when thunder.client.getAccessToken() fails, and then have the
UI render that error instead of always showing “Loading…” when token.isEmpty().
Keep the fix localized to the token-loading state and the related render logic
so there is a visible failure path and retry-friendly behavior.
Comment on lines
+29
to
40
| suspend fun initiate( | ||
| applicationId: String, | ||
| flowType: FlowType, | ||
| ): EmbeddedFlowResponse { | ||
| val body = | ||
| mapOf( | ||
| "applicationId" to applicationId, | ||
| "flowType" to flowType.value, | ||
| "verbose" to true, | ||
| ) | ||
| return httpClient.post("/flow/execute", body, requiresAuth = false) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trailing comma on a call-site mapOf(...) argument list.
"verbose" to true, at Line 37 leaves a trailing comma inside the mapOf(...) call site. As per coding guidelines, Do not use a trailing comma on the last argument at call sites.
🔧 Proposed fix
val body =
mapOf(
"applicationId" to applicationId,
"flowType" to flowType.value,
- "verbose" to true,
+ "verbose" to true
)📝 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
| suspend fun initiate( | |
| applicationId: String, | |
| flowType: FlowType, | |
| ): EmbeddedFlowResponse { | |
| val body = | |
| mapOf( | |
| "applicationId" to applicationId, | |
| "flowType" to flowType.value, | |
| "verbose" to true, | |
| ) | |
| return httpClient.post("/flow/execute", body, requiresAuth = false) | |
| } | |
| suspend fun initiate( | |
| applicationId: String, | |
| flowType: FlowType, | |
| ): EmbeddedFlowResponse { | |
| val body = | |
| mapOf( | |
| "applicationId" to applicationId, | |
| "flowType" to flowType.value, | |
| "verbose" to true | |
| ) | |
| return httpClient.post("/flow/execute", body, requiresAuth = false) | |
| } |
🤖 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 `@src/main/kotlin/dev/thunderid/android/auth/FlowExecutionClient.kt` around
lines 29 - 40, The mapOf call inside FlowExecutionClient.initiate has a trailing
comma on the last argument, which violates the call-site formatting rule. Remove
the trailing comma after the "verbose" to true entry in the body map
construction, keeping the rest of initiate and the
httpClient.post("/flow/execute", ...) call unchanged.
Source: Coding guidelines
Comment on lines
+48
to
+62
| suspend fun refresh(clientId: String): TokenResponse = | ||
| mutex.withLock { | ||
| val refreshToken = | ||
| tokenStore.refreshToken() | ||
| ?: throw IAMException(ThunderIDErrorCode.SESSION_EXPIRED, "No refresh token available") | ||
| val body = | ||
| mapOf( | ||
| "grant_type" to "refresh_token", | ||
| "refresh_token" to refreshToken, | ||
| "client_id" to clientId, | ||
| ) | ||
| val response: TokenResponse = httpClient.post("/oauth2/token", body, requiresAuth = false) | ||
| tokenStore.save(response) | ||
| response | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trailing comma on a call-site mapOf(...) argument list.
"client_id" to clientId, at Line 57 leaves a trailing comma inside a mapOf(...) invocation, which is a call site, not a declaration. As per coding guidelines, Do not use a trailing comma on the last argument at call sites. while Use a trailing comma on the last element of multi-line declarations such as function parameters and enum entries. This would fail ktlintCheck.
🔧 Proposed fix
val body =
mapOf(
"grant_type" to "refresh_token",
"refresh_token" to refreshToken,
- "client_id" to clientId,
+ "client_id" to clientId,
)Note: remove the trailing comma from the last call-site argument:
- "client_id" to clientId,
+ "client_id" to clientId🤖 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 `@src/main/kotlin/dev/thunderid/android/token/TokenRefresher.kt` around lines
48 - 62, The TokenRefresher.refresh method builds a call-site mapOf(...) request
body with a trailing comma on the last entry, which violates the ktlint
call-site comma rule. Remove the trailing comma from the final entry in the body
map inside refresh so the OAuth token refresh request still uses the same keys
and values but passes ktlintCheck.
Source: Coding guidelines
Comment on lines
+177
to
+181
| android.util.Log.d( | ||
| "ThunderID:SignIn", | ||
| "status=${response.flowStatus} inputs=${response.data?.inputs?.size} " + | ||
| "actions=${response.data?.actions?.size} data=${response.data}", | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid logging full response payload.
android.util.Log.d(..., "... data=${response.data}") logs the entire FlowStepData (inputs, actions, and arbitrary meta map) at debug level. This flow-step data can carry sensitive/PII-adjacent content (e.g., email hints, security-question metadata) and would be readable via adb logcat on any device where debug logging isn't stripped. Restrict the log to non-sensitive counts only (as the line-range description intended), and drop the data=${response.data} portion.
🔒 Proposed fix
android.util.Log.d(
"ThunderID:SignIn",
- "status=${response.flowStatus} inputs=${response.data?.inputs?.size} " +
- "actions=${response.data?.actions?.size} data=${response.data}",
+ "status=${response.flowStatus} inputs=${response.data?.inputs?.size} " +
+ "actions=${response.data?.actions?.size}",
)📝 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
| android.util.Log.d( | |
| "ThunderID:SignIn", | |
| "status=${response.flowStatus} inputs=${response.data?.inputs?.size} " + | |
| "actions=${response.data?.actions?.size} data=${response.data}", | |
| ) | |
| android.util.Log.d( | |
| "ThunderID:SignIn", | |
| "status=${response.flowStatus} inputs=${response.data?.inputs?.size} " + | |
| "actions=${response.data?.actions?.size}", | |
| ) |
🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt`
around lines 177 - 181, The debug log in SignIn’s response handling is leaking
the full FlowStepData payload via response.data. Update the android.util.Log.d
call in SignIn to log only non-sensitive summary information such as
response.flowStatus and the sizes of response.data?.inputs and
response.data?.actions, and remove the data=${response.data} portion entirely.
Comment on lines
+181
to
+186
| FlowStatus.COMPLETE -> { | ||
| thunderState.refresh() | ||
| onComplete?.invoke() | ||
| } | ||
| FlowStatus.PROMPT_ONLY -> state.update(response) | ||
| FlowStatus.INCOMPLETE -> {} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Outline: SignUp.kt\n'
ast-grep outline src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt --view expanded || true
printf '\n## Outline: SignIn.kt\n'
ast-grep outline src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.kt --view expanded || true
printf '\n## Outline: Models.kt\n'
ast-grep outline src/main/kotlin/dev/thunderid/compose/components/presentation/auth/Models.kt --view expanded || true
printf '\n## Relevant symbols and status handling\n'
rg -n -A4 -B4 'FlowStatus|handleSignUpResponse|handleSignInResponse|signUp\(|signIn\(' \
src/main/kotlin/dev/thunderid/compose/components/presentation/authRepository: thunder-id/android-sdks
Length of output: 15555
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## FlowStatus definition\n'
rg -n -A6 -B6 'enum class FlowStatus|sealed class FlowStatus|class FlowStatus|FlowStatus' src main . --glob '!**/build/**' --glob '!**/dist/**' --glob '!**/node_modules/**'
printf '\n## signUp / signIn response docs and call sites\n'
rg -n -A4 -B4 'signUp\(|signIn\(|PROMPT_ONLY|INCOMPLETE|EmbeddedFlowResponse|EmbeddedSignInPayload|EmbeddedFlowRequestConfig' \
src/main/kotlin
printf '\n## SignUp state update implementation\n'
sed -n '1,260p' src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt
printf '\n## SignIn state update implementation\n'
sed -n '1,260p' src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.ktRepository: thunder-id/android-sdks
Length of output: 21467
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Models.kt around FlowStatus and response types\n'
sed -n '80,120p' src/main/kotlin/dev/thunderid/android/Models.kt
printf '\n## ThunderIDClient signUp path\n'
rg -n -A30 -B10 'fun signUp|signUp\(' src/main/kotlin/dev/thunderid/android/ThunderIDClient.kt
printf '\n## SignUpState update body\n'
sed -n '52,95p' src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt
printf '\n## SignInState update body\n'
sed -n '58,100p' src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignIn.ktRepository: thunder-id/android-sdks
Length of output: 5711
Handle FlowStatus.INCOMPLETE like the other intermediate sign-up states.
SignUpState.update(response) is the only path that refreshes flowId, challengeToken, inputs, and actions. Leaving INCOMPLETE as a no-op can freeze a multi-step registration on the previous form, while SignIn.kt already treats INCOMPLETE as a refreshable intermediate state.
Proposed fix
- FlowStatus.PROMPT_ONLY -> state.update(response)
- FlowStatus.INCOMPLETE -> {}
+ FlowStatus.PROMPT_ONLY, FlowStatus.INCOMPLETE -> state.update(response)📝 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
| FlowStatus.COMPLETE -> { | |
| thunderState.refresh() | |
| onComplete?.invoke() | |
| } | |
| FlowStatus.PROMPT_ONLY -> state.update(response) | |
| FlowStatus.INCOMPLETE -> {} | |
| FlowStatus.COMPLETE -> { | |
| thunderState.refresh() | |
| onComplete?.invoke() | |
| } | |
| FlowStatus.PROMPT_ONLY, FlowStatus.INCOMPLETE -> state.update(response) |
🤖 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 `@src/main/kotlin/dev/thunderid/compose/components/presentation/auth/SignUp.kt`
around lines 181 - 186, `SignUp` currently leaves `FlowStatus.INCOMPLETE` as a
no-op, so the state never refreshes for multi-step registration. Update the
`when` branch in `SignUp.kt` near the `FlowStatus.COMPLETE`/`PROMPT_ONLY`
handling to treat `INCOMPLETE` the same as the other intermediate states by
calling `state.update(response)` (matching the `SignIn` flow behavior), while
keeping `COMPLETE` as the completion path.
Comment on lines
+72
to
+76
| modifier = | ||
| Modifier.defaultMinSize(minHeight = 44.dp).clickable { | ||
| signOut() | ||
| onSignOutComplete?.invoke() | ||
| }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Don't fire onSignOutComplete until logout finishes.
signOut() only starts the coroutine, so this callback now runs before the client sign-out and refresh complete. Callers can navigate away while the old auth state is still live.
🤖 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
`@src/main/kotlin/dev/thunderid/compose/components/presentation/user/UserDropdown.kt`
around lines 72 - 76, The UserDropdown sign-out click handler is invoking
onSignOutComplete immediately after signOut(), but signOut() only starts the
logout coroutine. Update the sign-out flow in UserDropdown so the completion
callback runs only after the client sign-out and refresh logic has finished,
likely by moving the callback into the end of the logout completion path in
signOut() or by making the click handler await the logout result before calling
onSignOutComplete.
Comment on lines
+80
to
+93
| fun `getConfiguration returns config after init`() = | ||
| runTest { | ||
| val config = | ||
| ThunderIDConfig( | ||
| baseUrl = "https://localhost:8090", | ||
| clientId = "my-client", | ||
| scopes = listOf("openid", "profile"), | ||
| ) | ||
| client.initialize(config, storage) | ||
| val retrieved = client.getConfiguration() | ||
| assertEquals("https://localhost:8090", retrieved.baseUrl) | ||
| assertEquals("my-client", retrieved.clientId) | ||
| assertEquals(listOf("openid", "profile"), retrieved.scopes) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Trailing comma on a call-site ThunderIDConfig(...) constructor invocation.
scopes = listOf("openid", "profile"), at Line 86 is the last argument at a call site (constructing ThunderIDConfig), not a declaration. As per coding guidelines, Do not use a trailing comma on the last argument at call sites.
🔧 Proposed fix
val config =
ThunderIDConfig(
baseUrl = "https://localhost:8090",
clientId = "my-client",
- scopes = listOf("openid", "profile"),
+ scopes = listOf("openid", "profile")
)📝 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
| fun `getConfiguration returns config after init`() = | |
| runTest { | |
| val config = | |
| ThunderIDConfig( | |
| baseUrl = "https://localhost:8090", | |
| clientId = "my-client", | |
| scopes = listOf("openid", "profile"), | |
| ) | |
| client.initialize(config, storage) | |
| val retrieved = client.getConfiguration() | |
| assertEquals("https://localhost:8090", retrieved.baseUrl) | |
| assertEquals("my-client", retrieved.clientId) | |
| assertEquals(listOf("openid", "profile"), retrieved.scopes) | |
| } | |
| fun `getConfiguration returns config after init`() = | |
| runTest { | |
| val config = | |
| ThunderIDConfig( | |
| baseUrl = "https://localhost:8090", | |
| clientId = "my-client", | |
| scopes = listOf("openid", "profile") | |
| ) | |
| client.initialize(config, storage) | |
| val retrieved = client.getConfiguration() | |
| assertEquals("https://localhost:8090", retrieved.baseUrl) | |
| assertEquals("my-client", retrieved.clientId) | |
| assertEquals(listOf("openid", "profile"), retrieved.scopes) | |
| } |
🤖 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 `@src/test/kotlin/dev/thunderid/android/ThunderIDClientTest.kt` around lines 80
- 93, The ThunderIDConfig call in ThunderIDClientTest uses an invalid trailing
comma on the last constructor argument at a call site. Update the
`getConfiguration returns config after init` test so the `ThunderIDConfig(...)`
invocation no longer ends its final `scopes` argument with a comma, while
keeping the rest of the setup and assertions unchanged.
Source: Coding guidelines
brionmario
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.github/workflows/release.yml:
- Around line 82-90: The version bump logic in the release workflow assumes
`build.gradle.kts` contains a plain numeric `X.Y.Z` version, so `IFS='.' read -r
major minor patch` and the arithmetic update can mis-handle suffixes like
`-SNAPSHOT`. Update the version parsing block in the release job to validate the
`current` value from `build.gradle.kts` before splitting or incrementing it, and
fail fast with a clear error when the format is not exactly numeric. Keep the
fix localized to the release step that uses `current`, `major`, `minor`,
`patch`, and `new_version`.
- Around line 24-25: The workflow currently grants `contents: write` at the top
level, which applies it to both `validate` and `release`. Move the permissions
setting into the `release` job only so `validate` stays read-only, and keep the
job names and `permissions` block aligned in `.github/workflows/release.yml`
when updating the workflow structure.
- Around line 49-52: The checkout step is persisting the custom token in git
config, which leaves credentials available to later jobs. Update the workflow’s
checkout configuration to use persist-credentials false, and adjust the push
logic in the release job to inject the token only at the push step by explicitly
configuring the git remote/auth there. Use the existing checkout step and the
later push step in this release workflow as the points to change.
- Around line 105-106: The release workflow tag push is outside the existing
retry logic and doesn’t guard against re-runs, so update the release job to
handle tag creation/push with the same resilience as the commit push. In the
workflow around the tag commands, add an idempotency check for the version tag
before bumping or creating it, and wrap the git tag/git push steps in the same
retry pattern used for the commit push. Use the existing bump output and tag
push steps in .github/workflows/release.yml as the main points to adjust.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 93ba52bd-460a-4d7c-bbe3-907578121ce5
📒 Files selected for processing (2)
.github/workflows/release.ymljitpack.yml
✅ Files skipped from review due to trivial changes (1)
- jitpack.yml
Comment on lines
+24
to
+25
| permissions: | ||
| contents: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scope contents: write to the release job only.
The validate job needs no write permissions, but the workflow-level permissions: contents: write grants it to every job. Move the permission block into the release job to follow least-privilege.
🛡️ Proposed fix
permissions:
- contents: write
+ # No workflow-level permissions; each job declares its own.
env:
@@ -43,6 +43,8 @@
release:
name: 🤖 Release android
needs: [validate]
if: needs.validate.result == 'success'
runs-on: ubuntu-latest
+ permissions:
+ contents: write
steps:🧰 Tools
🪛 zizmor (1.26.1)
[error] 25-25: overly broad permissions (excessive-permissions): contents: write is overly broad at the workflow level
(excessive-permissions)
🤖 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 @.github/workflows/release.yml around lines 24 - 25, The workflow currently
grants `contents: write` at the top level, which applies it to both `validate`
and `release`. Move the permissions setting into the `release` job only so
`validate` stays read-only, and keep the job names and `permissions` block
aligned in `.github/workflows/release.yml` when updating the workflow structure.
Source: Linters/SAST tools
Comment on lines
+49
to
+52
| - name: 📥 Checkout Code | ||
| uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 | ||
| with: | ||
| token: ${{ secrets.THUNDERID_AUTOMATION_BOT }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Set persist-credentials: false and configure git auth explicitly before push.
The checkout action with a custom token persists it in .git/config by default, making it readable by all subsequent steps (ktlint, test, build). Set persist-credentials: false and configure the git remote URL with the token only in the push step.
🛡️ Proposed fix
- name: 📥 Checkout Code
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
token: ${{ secrets.THUNDERID_AUTOMATION_BOT }}
+ persist-credentials: falseThen in the push step, configure the remote URL with the token:
- name: 📤 Commit, Tag & Push
+ env:
+ GH_TOKEN: ${{ secrets.THUNDERID_AUTOMATION_BOT }}
run: |
git config user.name "${{ env.RELEASE_GIT_USER_NAME }}"
git config user.email "${{ env.RELEASE_GIT_USER_EMAIL }}"
git add build.gradle.kts
git diff --cached --quiet || git commit -m "[Release] android v${{ steps.bump.outputs.version }}"
+ git remote set-url origin "https://x-access-token:${GH_TOKEN}`@github.com/`${{ github.repository }}.git"
for i in $(seq 1 5); do🧰 Tools
🪛 zizmor (1.26.1)
[warning] 49-52: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/release.yml around lines 49 - 52, The checkout step is
persisting the custom token in git config, which leaves credentials available to
later jobs. Update the workflow’s checkout configuration to use
persist-credentials false, and adjust the push logic in the release job to
inject the token only at the push step by explicitly configuring the git
remote/auth there. Use the existing checkout step and the later push step in
this release workflow as the points to change.
Source: Linters/SAST tools
Comment on lines
+82
to
+90
| current=$(grep -m1 '^version = ' build.gradle.kts | sed -E 's/version = "(.*)"/\1/') | ||
| IFS='.' read -r major minor patch <<< "$current" | ||
| case "${{ github.event.inputs.bump_type }}" in | ||
| major) major=$((major + 1)); minor=0; patch=0 ;; | ||
| minor) minor=$((minor + 1)); patch=0 ;; | ||
| patch) patch=$((patch + 1)) ;; | ||
| esac | ||
| new_version="$major.$minor.$patch" | ||
| sed -i "s/version = \"$current\"/version = \"$new_version\"/g" build.gradle.kts |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Version parsing assumes pure numeric X.Y.Z format.
The IFS='.' read -r major minor patch and arithmetic increment will silently produce wrong results if the version contains a pre-release suffix (e.g., 0.0.0-SNAPSHOT), since bash arithmetic would interpret 0-SNAPSHOT as 0 - 0. Consider validating the format before parsing.
🔧 Proposed fix: validate version format
current=$(grep -m1 '^version = ' build.gradle.kts | sed -E 's/version = "(.*)"/\1/')
+ if ! [[ "$current" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
+ echo "❌ Version '$current' is not in X.Y.Z format. Aborting." >&2
+ exit 1
+ fi
IFS='.' read -r major minor patch <<< "$current"📝 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
| current=$(grep -m1 '^version = ' build.gradle.kts | sed -E 's/version = "(.*)"/\1/') | |
| IFS='.' read -r major minor patch <<< "$current" | |
| case "${{ github.event.inputs.bump_type }}" in | |
| major) major=$((major + 1)); minor=0; patch=0 ;; | |
| minor) minor=$((minor + 1)); patch=0 ;; | |
| patch) patch=$((patch + 1)) ;; | |
| esac | |
| new_version="$major.$minor.$patch" | |
| sed -i "s/version = \"$current\"/version = \"$new_version\"/g" build.gradle.kts | |
| current=$(grep -m1 '^version = ' build.gradle.kts | sed -E 's/version = "(.*)"/\1/') | |
| if ! [[ "$current" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then | |
| echo "❌ Version '$current' is not in X.Y.Z format. Aborting." >&2 | |
| exit 1 | |
| fi | |
| IFS='.' read -r major minor patch <<< "$current" | |
| case "${{ github.event.inputs.bump_type }}" in | |
| major) major=$((major + 1)); minor=0; patch=0 ;; | |
| minor) minor=$((minor + 1)); patch=0 ;; | |
| patch) patch=$((patch + 1)) ;; | |
| esac | |
| new_version="$major.$minor.$patch" | |
| sed -i "s/version = \"$current\"/version = \"$new_version\"/g" build.gradle.kts |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 84-84: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/release.yml around lines 82 - 90, The version bump logic
in the release workflow assumes `build.gradle.kts` contains a plain numeric
`X.Y.Z` version, so `IFS='.' read -r major minor patch` and the arithmetic
update can mis-handle suffixes like `-SNAPSHOT`. Update the version parsing
block in the release job to validate the `current` value from `build.gradle.kts`
before splitting or incrementing it, and fail fast with a clear error when the
format is not exactly numeric. Keep the fix localized to the release step that
uses `current`, `major`, `minor`, `patch`, and `new_version`.
Comment on lines
+105
to
+106
| git tag "v${{ steps.bump.outputs.version }}" | ||
| git push origin "v${{ steps.bump.outputs.version }}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Tag push lacks retry and idempotency check.
The commit push has a retry loop (lines 99–104), but the tag creation and push are outside it. If the tag push fails (e.g., race condition or transient network error), there is no retry. Additionally, re-running the workflow after a partial failure (commit pushed but tag not created) will bump the version again instead of detecting the existing tag.
🔧 Proposed fix: add tag existence check and retry
git tag "v${{ steps.bump.outputs.version }}"
- git push origin "v${{ steps.bump.outputs.version }}"
+ for i in $(seq 1 5); do
+ git push origin "v${{ steps.bump.outputs.version }}" && break
+ [ "$i" -eq 5 ] && exit 1
+ sleep $((i * 3))
+ doneFor idempotency, add a pre-check before the bump step:
- name: 🏷️ Bump Version
id: bump
run: |
+ if git tag --list "v$(grep -m1 '^version = ' build.gradle.kts | sed -E 's/version = "(.*)"/\1/')" | grep -q .; then
+ echo "Tag already exists for current version. Aborting." >&2
+ exit 1
+ fi
current=$(grep -m1 '^version = ' build.gradle.kts | sed -E 's/version = "(.*)"/\1/')🧰 Tools
🪛 zizmor (1.26.1)
[info] 105-105: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 105-105: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/release.yml around lines 105 - 106, The release workflow
tag push is outside the existing retry logic and doesn’t guard against re-runs,
so update the release job to handle tag creation/push with the same resilience
as the commit push. In the workflow around the tag commands, add an idempotency
check for the version tag before bumping or creating it, and wrap the git
tag/git push steps in the same retry pattern used for the commit push. Use the
existing bump output and tag push steps in .github/workflows/release.yml as the
main points to adjust.
savindi7
approved these changes
Jul 8, 2026
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
2 participants
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.
Purpose
Approach
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation / Chores
config.properties, refreshed build/release automation, and modernized Gradle wrapper + sample app resources.