make GitHub release author and asset uploader fields nullable#446
Merged
rainxchzed merged 4 commits intomainfrom Apr 22, 2026
Merged
make GitHub release author and asset uploader fields nullable#446rainxchzed merged 4 commits intomainfrom
rainxchzed merged 4 commits intomainfrom
Conversation
- Update `GithubAsset` and `GithubRelease` domain models to allow null values for `uploader` and `author`. - Modify `AssetNetwork` and `ReleaseNetwork` DTOs to handle optional `uploader` and `author` fields from API responses. - Update data and presentation mappers to safely handle null user objects using null-safe calls.
- Introduce `ReleasesStatusCard` component to handle failed, retrying, and empty release states in the repository details header. - Implement `RetryReleases` action and logic in `DetailsViewModel` to allow manual recovery when release fetching fails. - Update `DetailsState` to track release loading failures and retry status. - Add localized strings for release loading errors and empty states. - Enhance `DetailsRepositoryImpl` to serve stale cache data upon serialization failures before throwing an exception. - Refactor the details header to conditionally display the status card instead of version pickers and install buttons when releases are unavailable.
Introduced a backend-first strategy for fetching repository details, releases, READMEs, and user profiles. The application now prioritizes backend API calls with aggressive caching and only falls back to direct GitHub API requests during backend infrastructure failures (5xx errors or network timeouts).
- **Backend Integration**:
- Added `GithubReadmeResponseDto` and updated `BackendApiClient` to support new endpoints for releases, READMEs, and user profiles.
- Updated `BackendException` to include HTTP status codes for better error handling.
- Implemented a 15-second timeout for cold-path backend requests (READMEs and releases).
- **Data Repository**:
- Updated `DetailsRepositoryImpl` to use the backend-first logic.
- Added `shouldFallbackToGithub` logic to distinguish between actionable 4xx errors (which return stale cache or propagate) and infrastructure errors that trigger a fallback.
- Implemented Base64 decoding and preprocessing for README content fetched from the backend.
- **Mappers & Refactoring**:
- Centralized user profile mapping into `toDomainProfile`.
- Improved cache handling by serving stale data when backend requests fail with 4xx errors.
Contributor
WalkthroughBackend-first GitHub releases support: nullable uploader/author fields across DTO/domain/UI, new backend API endpoints (releases/readme/user), updated repository fallback logic (backend → stale cache → direct GitHub), and UI state/actions + a ReleasesStatusCard for retry/empty/failed flows. Changes
Sequence DiagramsequenceDiagram
participant UI as Details UI
participant VM as DetailsViewModel
participant Repo as DetailsRepository
participant API as BackendApiClient
participant GH as GitHub API
participant Cache as Cache Store
UI->>VM: Initial Load or RetryReleases
VM->>Repo: getAllReleases(owner,name)
Repo->>API: getReleases(owner,name)
alt Backend Success
API->>Repo: ReleaseNetwork[]
Repo->>Repo: map to Domain (nullable author)
Repo->>VM: GithubRelease[]
VM->>UI: update state (allReleases, selectedRelease)
else Backend 4xx
API->>Repo: BackendException(4xx)
Repo->>Cache: read stale releases
alt Stale found
Cache->>Repo: GithubRelease[] (stale)
Repo->>VM: stale releases + success flag
VM->>UI: display stale releases
else No stale
Repo->>VM: empty list + failure flag
VM->>UI: show ReleasesStatusCard (FAILED)
end
else Backend 5xx / Network Error
API->>Repo: BackendException(5xx) / network error
Repo->>GH: fetch releases from GitHub
GH->>Repo: GithubRelease[]
Repo->>VM: GithubRelease[]
VM->>UI: update state
end
alt User taps Retry
UI->>VM: RetryReleases
VM->>VM: set isRetryingReleases=true
VM->>UI: show ReleasesStatusCard (RETRYING)
VM->>Repo: getAllReleases(...) %% flow repeats
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt (1)
1989-2003:⚠️ Potential issue | 🟡 MinorRate-limited initial load leaves
releasesLoadFailed = false, so the retry card never appears.When
RateLimitExceptionis hit,allReleasesDeferredresolves to(emptyList(), true)as expected, but the outer rate-limit short-circuit at lines 2085‑2088 returns early before the state update at 2103‑2128 that would propagatereleasesLoadFailed = true. Combined witherrorMessage = null, this means the Header will renderReleasesStatus.EMPTY("No releases published yet") for a rate-limited user with no releases in cache — a misleading empty-state and no Retry affordance.Consider setting
releasesLoadFailed = true(or a rate-limit-specific flag) on the rate-limit early-return path so the UI shows the correct status card.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt` around lines 1989 - 2003, In DetailsViewModel's async block handling allReleasesDeferred, the RateLimitException branch sets rateLimited.set(true) but the outer early-return occurs before updating the UI state that drives releasesLoadFailed; update the rate-limit early-return path to also set releasesLoadFailed = true (or a dedicated rateLimitFailed flag consumed by the Header) before returning so that ReleasesStatus shows a retry/error card instead of EMPTY; locate the RateLimitException catch, the rateLimited.set(true) call, and the releasesLoadFailed state mutation to apply this change.feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt (1)
473-514:⚠️ Potential issue | 🟡 Minor
getRepoStatsstill uses the pre-PRgetOrNull()pattern — intentional or an oversight?Every other backend-first method in this PR (
getRepositoryByOwnerAndName,getAllReleases,getReadme,getUserProfile) was switched tofold(...)+shouldFallbackToGithubso that backend 4xx distinctly serves stale cache or propagates, while backend 5xx/infra errors fall through.getRepoStatsat lines 477‑514 still silently swallows any backend failure via.getOrNull()and falls through to GitHub on every error class, includingCancellationException.If this is intentional (stats is supplementary and GitHub enrichment is always desired), it's worth a comment. Otherwise, consider aligning this path with the other four for consistency and to avoid swallowing cancellation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt` around lines 473 - 514, The current backend call in getRepoStats uses backendApiClient.getRepo(owner, repo).getOrNull() which swallows all failures (including CancellationException) and always falls back to GitHub; change it to use the same fold(...) + shouldFallbackToGithub pattern used by getRepositoryByOwnerAndName/getAllReleases/getReadme/getUserProfile so 4xx backend errors serve stale cache or propagate and only appropriate errors fall through to GitHub enrichment. Specifically, replace the getOrNull() usage with backendApiClient.getRepo(owner, repo).fold(onSuccess = { backendRepo -> ... }, onFailure = { err -> if (shouldFallbackToGithub(err)) { /* run githubInfo path */ } else throw err }) while preserving the existing githubInfo enrichment, propagation of CancellationException, and subsequent cacheManager.put(cacheKey, result, REPO_STATS) logic that constructs the RepoStats.
🧹 Nitpick comments (4)
feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt (2)
61-74:shouldFallbackToGithubhas a hidden side effect — it throws.Despite the name and
Booleanreturn type, this function can throw (CancellationException), which is easy to miss at call sites. Consider either renaming (e.g.shouldFallbackToGithubOrRethrow) or inlining the CE check at each call site and keeping this helper side-effect-free. Also worth documenting: anything that is aBackendExceptionwith a non-5xx status (including429 Too Many Requestsor408 Request Timeout) will short-circuit the GitHub fallback — if those statuses are expected from the backend, confirm that's the desired routing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt` around lines 61 - 74, The helper shouldFallbackToGithub currently rethrows CancellationException which is a hidden side-effect; either remove the throw from shouldFallbackToGithub so it purely returns a Boolean and handle CancellationException at each call site before invoking shouldFallbackToGithub (inline the CE check), or rename the function to shouldFallbackToGithubOrRethrow and update all call sites to reflect the rethrowing behavior; also add a brief doc comment to shouldFallbackToGithub/shouldFallbackToGithubOrRethrow clarifying that BackendException with non-5xx status (e.g., 429/408) will prevent fallback so reviewers can verify that routing is intended.
409-430: Replace manual whitespace stripping withBase64.Mimedecoder; narrow exception handling.Two improvements in
processReadmeFromBackend:
- The current manual
replace("\n", "").replace("\r", "")handles only two whitespace characters.Base64.Default.decodeis strict and throws on any other whitespace (spaces, tabs) that the proxy may introduce. UseBase64.Mime.decodeinstead, which tolerates all whitespace.catch (e: Throwable)captures fatal errors likeOutOfMemoryError. Narrow toIllegalArgumentException, the documented exception forBase64.decode().Suggested change
- // GitHub's contents API base64-encodes with embedded newlines. - val raw = dto.content.replace("\n", "").replace("\r", "") - val decoded = try { - Base64.Default.decode(raw).decodeToString() - } catch (e: Throwable) { + // GitHub's contents API base64-encodes with embedded newlines + // (and potentially other whitespace introduced by the proxy). + val decoded = try { + Base64.Mime.decode(dto.content).decodeToString() + } catch (e: IllegalArgumentException) { logger.warn("Failed to base64-decode backend readme for $owner/$repo: ${e.message}") return null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt` around lines 409 - 430, In processReadmeFromBackend, stop manually stripping newlines and instead decode the README content with Base64.Mime.decode(raw) to tolerate all whitespace the proxy may add, and narrow the exception handler from catch (e: Throwable) to catch (e: IllegalArgumentException) so only decode errors are caught; update the existing logger.warn call inside that catch to reference the exception (e.message) as it already does.core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt (2)
97-97: Redundantmessageon everyBackendExceptionconstruction.Since
statusCodeis now a typed field onBackendException, passing"HTTP ${response.status.value}"as the message at every call site just echoes the same integer twice and adds boilerplate. Consider either deriving the message insideBackendException(e.g. defaultmessage = "HTTP $statusCode") or dropping the parameter entirely so callers just pass the status code.♻️ Proposed simplification
-class BackendException( - val statusCode: Int, - message: String, -) : Exception(message) +class BackendException( + val statusCode: Int, + message: String = "HTTP $statusCode", +) : Exception(message)Then call sites become
BackendException(response.status.value).Also applies to: 107-107, 131-131, 157-157, 178-178, 203-203, 223-223, 236-236, 252-252, 271-274
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt` at line 97, Call sites in BackendApiClient that construct BackendException with Result.failure(BackendException(response.status.value, "HTTP ${response.status.value}")) are redundant; change BackendException to accept a single statusCode parameter and derive a default message (e.g., message = "HTTP $statusCode") or provide a defaulted message parameter so callers can pass only response.status.value. Update BackendException's constructor/signature accordingly and replace the multiple call sites in BackendApiClient (the Result.failure(...) lines) to use BackendException(response.status.value) only.
227-238: Consider an extended timeout ongetUserfor consistency with other cold paths.
getReleasesandgetReadmeboth override the default 5 s request/socket timeout to 15 s because the backend has to round-trip to GitHub on cold cache.getUsergoes through the same backend-to-GitHub cold path but relies on the default 5 s, which will likely truncate legitimate requests on first hit. Either apply the same 15 s override or document why this endpoint is expected to stay warm.♻️ Proposed change
suspend fun getUser(username: String): Result<UserProfileNetwork> = safeCall { val token = currentUserGithubToken() val response = httpClient.get("user/$username") { if (token != null) header(X_GITHUB_TOKEN_HEADER, token) + timeout { + requestTimeoutMillis = 15_000 + socketTimeoutMillis = 15_000 + } }(Drop the token header per the guideline comment above.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt` around lines 227 - 238, getUser currently uses the default 5s request/socket timeout while getReleases and getReadme override to 15s for backend→GitHub cold trips; update getUser (function getUser in BackendApiClient) to apply the same 15s request/socket timeout override as those methods and remove the token header per the guideline (drop use of currentUserGithubToken() / X_GITHUB_TOKEN_HEADER in this call) so the endpoint has consistent timeout behavior with other cold paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt`:
- Around line 169-238: The four API methods getRepo, getReleases, getReadme, and
getUser are incorrectly attaching the user's GitHub token (via
currentUserGithubToken()/TokenStore) and must not forward X_GITHUB_TOKEN_HEADER;
remove the token lookup and the conditional header(...) call from each of these
functions (or switch them to a backend-only httpClient that has no TokenStore
access) so no X-GitHub-Token is sent on repo/..., releases/..., readme/... or
user/... requests.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt`:
- Around line 78-83: The releasesStatus computation can return
ReleasesStatus.EMPTY even when no repository is loaded; modify the logic that
sets releasesStatus (the when block computing releasesStatus) to first check
state.repository != null before evaluating release-specific conditions
(state.releasesLoadFailed, state.isRetryingReleases, !state.isLoading &&
state.allReleases.isEmpty()), and return null when state.repository is null so
the UI won't show EMPTY for missing repository (this mirrors the ViewModel guard
in retryReleases()).
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 512-538: retryReleases currently unconditionally sets
selectedReleaseCategory = ReleaseCategory.STABLE, losing the user's prior
choice; change retryReleases to preserve the previous category (val prevCategory
= _state.value.selectedReleaseCategory), attempt to pick a release from the
fetched releases that matches prevCategory (filter releases by category:
PRE_RELEASE, STABLE, or ALL), and only fall back to the existing STABLE-first
selection if no release exists in prevCategory; then set selectedReleaseCategory
to prevCategory unless the fallback path was used, compute assets via
recomputeAssetsForRelease(selected, _state.value.installedApp), and update
allReleases, selectedRelease, selectedReleaseCategory, installableAssets,
primaryAsset and flags as before.
- Around line 539-548: The catch block in DetailsViewModel's retry logic
currently swallows CancellationException by using catch (t: Throwable); update
that handler to rethrow CE before handling other throwables (e.g., add an
explicit check "if (t is CancellationException) throw t") so that structured
concurrency is preserved, then continue logging and updating _state
(isRetryingReleases/releasesLoadFailed) for non-cancellation errors; locate this
in the retry code path inside DetailsViewModel.kt where the current catch (t:
Throwable) is defined and apply the change.
---
Outside diff comments:
In
`@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt`:
- Around line 473-514: The current backend call in getRepoStats uses
backendApiClient.getRepo(owner, repo).getOrNull() which swallows all failures
(including CancellationException) and always falls back to GitHub; change it to
use the same fold(...) + shouldFallbackToGithub pattern used by
getRepositoryByOwnerAndName/getAllReleases/getReadme/getUserProfile so 4xx
backend errors serve stale cache or propagate and only appropriate errors fall
through to GitHub enrichment. Specifically, replace the getOrNull() usage with
backendApiClient.getRepo(owner, repo).fold(onSuccess = { backendRepo -> ... },
onFailure = { err -> if (shouldFallbackToGithub(err)) { /* run githubInfo path
*/ } else throw err }) while preserving the existing githubInfo enrichment,
propagation of CancellationException, and subsequent cacheManager.put(cacheKey,
result, REPO_STATS) logic that constructs the RepoStats.
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.kt`:
- Around line 1989-2003: In DetailsViewModel's async block handling
allReleasesDeferred, the RateLimitException branch sets rateLimited.set(true)
but the outer early-return occurs before updating the UI state that drives
releasesLoadFailed; update the rate-limit early-return path to also set
releasesLoadFailed = true (or a dedicated rateLimitFailed flag consumed by the
Header) before returning so that ReleasesStatus shows a retry/error card instead
of EMPTY; locate the RateLimitException catch, the rateLimited.set(true) call,
and the releasesLoadFailed state mutation to apply this change.
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt`:
- Line 97: Call sites in BackendApiClient that construct BackendException with
Result.failure(BackendException(response.status.value, "HTTP
${response.status.value}")) are redundant; change BackendException to accept a
single statusCode parameter and derive a default message (e.g., message = "HTTP
$statusCode") or provide a defaulted message parameter so callers can pass only
response.status.value. Update BackendException's constructor/signature
accordingly and replace the multiple call sites in BackendApiClient (the
Result.failure(...) lines) to use BackendException(response.status.value) only.
- Around line 227-238: getUser currently uses the default 5s request/socket
timeout while getReleases and getReadme override to 15s for backend→GitHub cold
trips; update getUser (function getUser in BackendApiClient) to apply the same
15s request/socket timeout override as those methods and remove the token header
per the guideline (drop use of currentUserGithubToken() / X_GITHUB_TOKEN_HEADER
in this call) so the endpoint has consistent timeout behavior with other cold
paths.
In
`@feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt`:
- Around line 61-74: The helper shouldFallbackToGithub currently rethrows
CancellationException which is a hidden side-effect; either remove the throw
from shouldFallbackToGithub so it purely returns a Boolean and handle
CancellationException at each call site before invoking shouldFallbackToGithub
(inline the CE check), or rename the function to shouldFallbackToGithubOrRethrow
and update all call sites to reflect the rethrowing behavior; also add a brief
doc comment to shouldFallbackToGithub/shouldFallbackToGithubOrRethrow clarifying
that BackendException with non-5xx status (e.g., 429/408) will prevent fallback
so reviewers can verify that routing is intended.
- Around line 409-430: In processReadmeFromBackend, stop manually stripping
newlines and instead decode the README content with Base64.Mime.decode(raw) to
tolerate all whitespace the proxy may add, and narrow the exception handler from
catch (e: Throwable) to catch (e: IllegalArgumentException) so only decode
errors are caught; update the existing logger.warn call inside that catch to
reference the exception (e.message) as it already does.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 031e3288-a291-4a02-bffe-bad3929bfd12
📒 Files selected for processing (17)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/AssetNetwork.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/GithubReadmeResponseDto.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/ReleaseNetwork.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/AssetNetwork.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/ReleaseNetwork.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubAsset.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/GithubRelease.ktcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/mappers/GithubAssetMapper.ktfeature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/model/GithubAssetUi.ktfeature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsAction.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsState.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/ReleasesStatusCard.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt
Comment on lines
169
to
238
| suspend fun getRepo(owner: String, name: String): Result<BackendRepoResponse> = | ||
| safeCall { | ||
| val response = httpClient.get("repo/$owner/$name") | ||
| val token = currentUserGithubToken() | ||
| val response = httpClient.get("repo/$owner/$name") { | ||
| if (token != null) header(X_GITHUB_TOKEN_HEADER, token) | ||
| } | ||
| if (response.status.isSuccess()) { | ||
| Result.success(response.body()) | ||
| } else { | ||
| Result.failure(BackendException(response.status.value, "HTTP ${response.status.value}")) | ||
| } | ||
| } | ||
|
|
||
| suspend fun getReleases( | ||
| owner: String, | ||
| name: String, | ||
| page: Int = 1, | ||
| perPage: Int = 100, | ||
| ): Result<List<ReleaseNetwork>> = | ||
| safeCall { | ||
| val token = currentUserGithubToken() | ||
| val response = httpClient.get("releases/$owner/$name") { | ||
| parameter("page", page) | ||
| parameter("per_page", perPage) | ||
| if (token != null) header(X_GITHUB_TOKEN_HEADER, token) | ||
| // Cold path: backend goes to GitHub + paginates. 15s covers p99. | ||
| timeout { | ||
| requestTimeoutMillis = 15_000 | ||
| socketTimeoutMillis = 15_000 | ||
| } | ||
| } | ||
| if (response.status.isSuccess()) { | ||
| Result.success(response.body()) | ||
| } else { | ||
| Result.failure(BackendException(response.status.value, "HTTP ${response.status.value}")) | ||
| } | ||
| } | ||
|
|
||
| suspend fun getReadme( | ||
| owner: String, | ||
| name: String, | ||
| ): Result<GithubReadmeResponseDto> = | ||
| safeCall { | ||
| val token = currentUserGithubToken() | ||
| val response = httpClient.get("readme/$owner/$name") { | ||
| if (token != null) header(X_GITHUB_TOKEN_HEADER, token) | ||
| timeout { | ||
| requestTimeoutMillis = 15_000 | ||
| socketTimeoutMillis = 15_000 | ||
| } | ||
| } | ||
| if (response.status.isSuccess()) { | ||
| Result.success(response.body()) | ||
| } else { | ||
| Result.failure(BackendException(response.status.value, "HTTP ${response.status.value}")) | ||
| } | ||
| } | ||
|
|
||
| suspend fun getUser(username: String): Result<UserProfileNetwork> = | ||
| safeCall { | ||
| val token = currentUserGithubToken() | ||
| val response = httpClient.get("user/$username") { | ||
| if (token != null) header(X_GITHUB_TOKEN_HEADER, token) | ||
| } | ||
| if (response.status.isSuccess()) { | ||
| Result.success(response.body()) | ||
| } else { | ||
| Result.failure(BackendException("HTTP ${response.status.value}")) | ||
| Result.failure(BackendException(response.status.value, "HTTP ${response.status.value}")) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
X-GitHub-Token must not be sent on non-search endpoints.
getRepo, getReleases, getReadme, and getUser all attach X-GitHub-Token via currentUserGithubToken(). Per the repository guideline, this header is only allowed on /v1/search and /v1/search/explore — the four new/updated endpoints (repo/..., releases/..., readme/..., user/...) must not forward the user's GitHub token. This leaks the user's OAuth token to backend routes that shouldn't need it and widens the blast radius if any of those paths log or proxy headers.
Drop the token lookup and header on these four methods, or route them through a distinct client that doesn't have TokenStore access.
🔒️ Proposed fix
suspend fun getRepo(owner: String, name: String): Result<BackendRepoResponse> =
safeCall {
- val token = currentUserGithubToken()
- val response = httpClient.get("repo/$owner/$name") {
- if (token != null) header(X_GITHUB_TOKEN_HEADER, token)
- }
+ val response = httpClient.get("repo/$owner/$name")
if (response.status.isSuccess()) {
Result.success(response.body())
} else {
Result.failure(BackendException(response.status.value, "HTTP ${response.status.value}"))
}
}
suspend fun getReleases(
owner: String,
name: String,
page: Int = 1,
perPage: Int = 100,
): Result<List<ReleaseNetwork>> =
safeCall {
- val token = currentUserGithubToken()
val response = httpClient.get("releases/$owner/$name") {
parameter("page", page)
parameter("per_page", perPage)
- if (token != null) header(X_GITHUB_TOKEN_HEADER, token)
// Cold path: backend goes to GitHub + paginates. 15s covers p99.
timeout {
requestTimeoutMillis = 15_000
socketTimeoutMillis = 15_000
}
}
...
}Apply the analogous removal to getReadme and getUser.
As per coding guidelines: "Never send X-GitHub-Token header on endpoints other than /v1/search and /v1/search/explore; keep TokenStore.currentToken() access private".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt`
around lines 169 - 238, The four API methods getRepo, getReleases, getReadme,
and getUser are incorrectly attaching the user's GitHub token (via
currentUserGithubToken()/TokenStore) and must not forward X_GITHUB_TOKEN_HEADER;
remove the token lookup and the conditional header(...) call from each of these
functions (or switch them to a backend-only httpClient that has no TokenStore
access) so no X-GitHub-Token is sent on repo/..., releases/..., readme/... or
user/... requests.
…ate consistency
- **Network/Backend**:
- Simplified `BackendException` by moving default error messages to the constructor.
- Added a 15-second request and socket timeout to user profile fetches.
- Refactored backend-to-GitHub fallback logic into `shouldFallbackToGithubOrRethrow` to ensure `CancellationException` is always propagated and 4xx errors (like rate limits) are treated as authoritative.
- Fixed README decoding by using `Base64.Mime` to handle embedded newlines more robustly.
- **Details Presentation**:
- Improved `DetailsViewModel` to preserve the user's selected release category (Stable/Pre-release/All) during retries.
- Updated rate-limit handling to explicitly set `releasesLoadFailed`, ensuring the UI shows a "Retry" affordance instead of an empty state.
- Added a guard in the `Header` component to prevent rendering release status states before the repository data has loaded.
- **Data/Repository**:
- Wrapped repository statistics fetching in a `fold` to properly handle backend failures, stale cache fallback, and GitHub enrichment.
- Ensured `RepoStats` preserves license and issue data from stale cache if GitHub enrichment fails.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt (1)
136-270: Verify thatDropdownMenuwas intentionally moved inside the newBox.Based on the brace structure,
DropdownMenu(lines 152–267) now sits inside theBox { … }that wrapsSmartInstallButton, but its indentation (12sp, same level asBox(on line 139) doesn't reflect that nesting — it reads as if it were a sibling ofBox, suggesting this may have been unintended when theBoxwrapper was added aroundSmartInstallButton.Since
DropdownMenuanchors to its parent's layout coordinates, this change also shifts the anchor from theitemslot to the innerBox(which hasfillMaxWidth()but no explicit positioning). If it's intentional (e.g., to anchor the menu to the install button), please reindent for clarity; otherwise, move it back out of theBox.Suggested reindent if placement inside
Boxis intentionalitem { val liquidState = LocalTopbarLiquidState.current Box( modifier = Modifier.fillMaxWidth(), ) { SmartInstallButton( ... ) - DropdownMenu( - expanded = state.isInstallDropdownExpanded, + DropdownMenu( + expanded = state.isInstallDropdownExpanded, ... - } - } + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt` around lines 136 - 270, The DropdownMenu was moved inside the Box wrapping SmartInstallButton but the indentation and braces make its nesting unclear and may be unintended; either move DropdownMenu back out to be a sibling of Box (so it anchors to the item slot) or keep it inside the Box and fix the braces/indentation to clearly nest DropdownMenu within Box. Locate the Box { ... } that contains SmartInstallButton and the DropdownMenu usage (references: Box, SmartInstallButton, DropdownMenu, state.isInstallDropdownExpanded, onAction) and adjust placement/indentation so the menu anchors to the intended parent and the code's brace structure matches the visual nesting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@feature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt`:
- Around line 136-270: The DropdownMenu was moved inside the Box wrapping
SmartInstallButton but the indentation and braces make its nesting unclear and
may be unintended; either move DropdownMenu back out to be a sibling of Box (so
it anchors to the item slot) or keep it inside the Box and fix the
braces/indentation to clearly nest DropdownMenu within Box. Locate the Box { ...
} that contains SmartInstallButton and the DropdownMenu usage (references: Box,
SmartInstallButton, DropdownMenu, state.isInstallDropdownExpanded, onAction) and
adjust placement/indentation so the menu anchors to the intended parent and the
code's brace structure matches the visual nesting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80597533-e6e9-4c32-84af-2b66d65fa0cb
📒 Files selected for processing (4)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktfeature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/DetailsViewModel.ktfeature/details/presentation/src/commonMain/kotlin/zed/rainxch/details/presentation/components/sections/Header.kt
✅ Files skipped from review due to trivial changes (2)
- feature/details/data/src/commonMain/kotlin/zed/rainxch/details/data/repository/DetailsRepositoryImpl.kt
- core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GithubAssetandGithubReleasedomain models to allow null values foruploaderandauthor.AssetNetworkandReleaseNetworkDTOs to handle optionaluploaderandauthorfields from API responses.Summary by CodeRabbit
New Features
Improvements