Skip to content

feat(forges): codeberg + forgejo source adapter (preview)#631

Open
rainxchzed wants to merge 9 commits into
mainfrom
feat/e8-codeberg-forgejo
Open

feat(forges): codeberg + forgejo source adapter (preview)#631
rainxchzed wants to merge 9 commits into
mainfrom
feat/e8-codeberg-forgejo

Conversation

@rainxchzed
Copy link
Copy Markdown
Member

@rainxchzed rainxchzed commented May 17, 2026

Summary

Sprint 3 Task #1 (E8). MVP foundation for multi-forge support.

Now works: Paste https://codeberg.org/owner/repo (or any Forgejo/Gitea host URL) in the manual-link sheet → GHS calls Forgejo /api/v1/... instead of GitHub, persists sourceHost, and polls the same host on every update cycle alongside GitHub repos.

Architecture

  • RepositorySource sealed interface (GitHub, Forgejo(host)) + RepositoryReference + RepositoryUrlParser in core/domain
  • ForgejoApiClient(host) reuses existing GitHub DTOs (Forgejo JSON is field-compatible) — anonymous reads only for now
  • ForgejoClientRegistry caches one client per host
  • InstalledApp.sourceHost: String? (null = GitHub legacy). DB v17 → v18, manual migration
  • InstalledAppsRepositoryImpl.fetchReleaseWindow branches to fetchForgejoReleaseWindow when sourceHost != nullUpdateCheckWorker picks Forgejo repos up unchanged
  • AppsRepositoryImpl.fetchRepoInfo + linkAppToRepo accept sourceHost and route appropriately

Out of scope (follow-up PRs)

  • In-app asset picker for Forgejo (tracker-only first PR — users install via browser link until next PR)
  • Search source filter UI
  • Tweaks "Add custom forge" hostname entry
  • Per-host OAuth / PAT tokens
  • Backend /v1/releases proxy support for Forgejo (currently direct HTTP)

Test plan

  • Paste https://codeberg.org/forgejo/forgejo in manual-link → app appears in library, lastCheckedAt updates on next cycle
  • DB v17 → v18 migrates on existing installs, sourceHost NULL for legacy rows
  • Pasting github.com URL still works identically (no regression)
  • Pasting non-github, non-known-forgejo, non-pattern-match URL → "Invalid URL" error (no crash)
  • UpdateCheckWorker picks up Codeberg-tracked rows + polls Forgejo API (verify via logs)

Summary by CodeRabbit

  • New Features

    • Preview support for Codeberg and Forgejo/Gitea: paste those URLs in the manual-link form; the app will track, download/install, and poll them like GitHub repos.
    • Tweaks → Network: add/manage custom self-hosted forge hosts.
  • Improvements

    • Manual-link flow recognizes non‑GitHub sources and threads source info through linking/updates.
    • Linked apps display a source chip in list and compact views.
    • Per‑section sorting continues to be preserved.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR adds preview support for Forgejo/Codeberg/Gitea sources alongside GitHub: database migration to v18 (sourceHost), domain types and URL parsing, a Ktor Forgejo client and registry, repository + DI wiring, apps linking/ViewModel changes, Tweaks UI for custom forges, UI SourceChip, and localized release notes/strings.

Changes

Forgejo/Codeberg Support

Layer / File(s) Summary
All changes (single checkpoint)
core/data/schemas/.../AppDatabase/18.json, core/data/src/.../db/*, core/data/src/.../network/*, core/domain/src/.../model/*, feature/apps/*, feature/tweaks/*, core/presentation/src/.../composeResources/*
Adds DB v18 and migration adding nullable sourceHost; domain models and RepoId codec; RepositoryUrlParser; ForgejoApiClient and ForgejoClientRegistry; wiring and repository changes to thread/persist sourceHost; AppsViewModel and UI changes to parse/link non-GitHub hosts; Tweaks custom-forges UI/state/actions and localized strings/release notes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 I hop from host to host, a tiny trail I chart,
Codeberg and Forgejo now have a place in my heart,
A chip upon the row shows where the source has been,
Releases polled and fetched — from self-hosted to GitHub’s scene,
Hooray — more hosts to track! 🌱

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly describes the main feature: adding Codeberg and Forgejo source adapter support. It matches the core objective of implementing multi-forge support for Forgejo/Gitea hosts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/e8-codeberg-forgejo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 17, 2026

Greptile Summary

This PR adds MVP multi-forge support, allowing users to paste Codeberg/Forgejo/Gitea URLs in the manual-link sheet. It introduces a new RepositorySource sealed interface, RepositoryUrlParser, ForgejoApiClient/ForgejoClientRegistry, a sourceHost column in installed_apps (DB v17→v18), and routes both the update-check worker and the link flow through Forgejo's /api/v1/… endpoints.

  • ForgejoClientRegistry now uses a Mutex and subscribes to ProxyManager.configFlow, resolving the two previously flagged thread-safety and proxy-bypass issues.
  • RepoIdCodec encodes Forgejo repo IDs as negative Longs to separate them from GitHub IDs, but only 15 bits are used for the host fingerprint, leaving a collision risk between different Forgejo instances with the same lower-15-bit hash and the same numeric repo ID.
  • Several carry-over gaps from prior review rounds remain open: updateSingleApp still routes Forgejo apps to the GitHub backend, exportApps/importNative drop sourceHost, and the advanced-filter preview fetches from GitHub for Forgejo-tracked apps.

Confidence Score: 3/5

Mergeable as a preview feature, but several data-integrity gaps remain: the RepoIdCodec 15-bit host hash can silently collide between Forgejo instances, and carry-over issues (update tap, export/import, filter preview) cause wrong-forge routing for Forgejo-tracked apps.

The RepoIdCodec introduces a new collision vector between different Forgejo instances that mirrors the original GitHub/Forgejo ID collision it was meant to fix. On top of that, multiple previously flagged bugs remain open — updateSingleApp always queries the GitHub backend for Forgejo apps (making the in-app update button broken for Codeberg-only repos), and exportApps/importNative silently drop sourceHost so a round-trip export destroys Forgejo tracking. Together these represent active data-correctness defects on the Forgejo code path.

RepoIdCodec.kt (hash width), AppsRepositoryImpl.kt (export/import sourceHost), and AppsViewModel.kt (updateSingleApp forge routing) all need attention before the feature graduates from preview.

Important Files Changed

Filename Overview
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt New Ktor client for Forgejo/Gitea API; timeouts (60s/30s/30s) and retry with exponential backoff are properly configured; re-uses existing DTOs.
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt Singleton client cache with Mutex for thread safety and proxy-config invalidation via StateFlow; both previously flagged issues (thread safety, proxy bypass) are resolved.
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepoIdCodec.kt Encodes Forgejo repo IDs as negative Longs to namespace-separate them from GitHub IDs; uses only 15 bits of hostname hash, creating a collision risk between different Forgejo instances.
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepositoryUrlParser.kt URL parser with heuristic Forgejo host detection; looksLikeForgejoHost has overly broad prefix matches (previously flagged) that misclassify well-known non-Forgejo services.
core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt Adds fetchForgejoReleaseWindow and routes to it when sourceHost != null; pre-release filtering correctly uses isEffectivelyPreRelease() via toDomain().
feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt Adds Forgejo routing to getLatestRelease, fetchRepoInfo, linkAppToRepo; export/import still drops sourceHost (previously flagged); pre-release filter inconsistency vs. InstalledAppsRepositoryImpl (previously flagged).
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt Manual-link flow correctly threads sourceHost through validation and linking; updateSingleApp still calls getLatestRelease without sourceHost (previously flagged); linkSourceHost not cleared in dismissLinkSheet/openLinkSheet.
feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt Adds custom Forgejo host CRUD actions; validation error message is hardcoded English instead of a string resource.
feature/search/data/src/commonMain/kotlin/zed/rainxch/search/data/repository/SearchRepositoryImpl.kt Adds forgejoSearch path; isFork = false hardcoded so fork repos won't display the fork badge; otherwise structurally sound.
core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/migrations/MIGRATION_17_18.kt Adds nullable sourceHost TEXT column to installed_apps via ALTER TABLE; existing rows correctly default to NULL (GitHub legacy).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User pastes URL] --> B[RepositoryUrlParser.parse]
    B -->|github.com| C[RepositorySource.GitHub\nsourceHost = null]
    B -->|known/custom\nForgejo host| D[RepositorySource.Forgejo\nsourceHost = host]
    B -->|looksLikeForgejoHost| D
    B -->|no match| E[null - Invalid URL error]

    C --> F[AppsRepositoryImpl\n.fetchRepoInfo / getLatestRelease\nvia BackendApiClient]
    D --> G[ForgejoClientRegistry\n.clientFor host]
    G --> H[ForgejoApiClient\nGET /api/v1/repos/owner/repo]

    F --> I[linkAppToRepo\nsourceHost = null]
    H --> I2[linkAppToRepo\nsourceHost = host]

    I --> J[(installed_apps\nsourceHost NULL)]
    I2 --> K[(installed_apps\nsourceHost = host)]

    J --> L[UpdateCheckWorker\nfetchReleaseWindow\nsourceHost=null - GitHub backend]
    K --> M[UpdateCheckWorker\nfetchReleaseWindow\nsourceHost!=null - ForgejoApiClient]
Loading

Fix All in Claude Code

Reviews (8): Last reviewed commit: "fix(secure): wrap all KSafe calls in cra..." | Re-trigger Greptile

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt`:
- Around line 35-37: ForgejoApiClient currently sets requestTimeoutMillis and
socketTimeoutMillis to HttpTimeoutConfig.INFINITE_TIMEOUT_MS; change both to a
finite 60_000 (60s) so metadata/release API calls use a 60s timeout while
leaving connectTimeoutMillis at 30_000; update the HttpClient/config block that
sets requestTimeoutMillis and socketTimeoutMillis (the code referencing
HttpTimeoutConfig.INFINITE_TIMEOUT_MS in ForgejoApiClient) to use 60_000
instead.

In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt`:
- Around line 4-8: The clients map is mutated unsafely by clientFor using
clients.getOrPut(...) which can race; change the backing store to a thread-safe
map (e.g., make clients a java.util.concurrent.ConcurrentHashMap<String,
ForgejoApiClient>) and replace the getOrPut usage in clientFor with
computeIfAbsent(key) { ForgejoApiClient(key) } (or alternatively guard the
getOrPut call with a synchronized lock) so concurrent calls to clientFor safely
create or return a single ForgejoApiClient instance.

In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepositoryUrlParser.kt`:
- Line 23: The repo name extraction in RepositoryUrlParser (variable repo)
strips the ".git" suffix case-sensitively; update the assignment in the code
that sets val repo = match.groupValues[3].removeSuffix(".git") to remove the
suffix case-insensitively — e.g. check endsWith(".git", ignoreCase = true) and
strip the last 4 characters or apply a case-insensitive regex replacement like
replace(Regex("(?i)\\.git$"), "") — so URLs ending with ".GIT" or other case
variants produce the correct repo name.
- Around line 26-30: The host classification in RepositoryUrlParser currently
returns null for common self-hosted domains like git.example.com; update the
detection so RepositorySource.Forgejo is chosen for hosts that are common
Forgejo/Gitea subdomains. Concretely, modify looksLikeForgejoHost (or the when
branch) to also return true when the host matches common subdomain prefixes
(e.g., startsWith "git.", "repo.", "source.", "scm.") or when the hostname
contains "gitea" or "forgejo" (in addition to existing knownForgejoHosts), so
that RepositorySource.Forgejo(host) is returned instead of null; ensure the
change references the existing symbols looksLikeForgejoHost, knownForgejoHosts,
and RepositorySource.Forgejo.
🪄 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: 14f58486-b708-4d06-be55-4d4e321c29b7

📥 Commits

Reviewing files that changed from the base of the PR and between 6632fd3 and eeff799.

📒 Files selected for processing (31)
  • core/data/schemas/zed.rainxch.core.data.local.db.AppDatabase/18.json
  • core/data/src/androidMain/kotlin/zed/rainxch/core/data/local/db/initDatabase.kt
  • core/data/src/androidMain/kotlin/zed/rainxch/core/data/local/db/migrations/MIGRATION_17_18.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/AppDatabase.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/local/db/entities/InstalledAppEntity.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/mappers/InstalledAppsMappers.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/InstalledAppsRepositoryImpl.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/InstalledApp.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/model/RepositorySource.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepositoryUrlParser.kt
  • core/presentation/src/commonMain/composeResources/files/whatsnew/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ar/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/bn/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/es/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/fr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/hi/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/it/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ja/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ko/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/pl/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ru/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/tr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/18.json
  • feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/di/SharedModule.kt
  • feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt
  • feature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsState.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt

Comment on lines +35 to +37
requestTimeoutMillis = HttpTimeoutConfig.INFINITE_TIMEOUT_MS
connectTimeoutMillis = 30_000
socketTimeoutMillis = HttpTimeoutConfig.INFINITE_TIMEOUT_MS
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect timeout configuration in API clients.
rg -n 'HttpTimeoutConfig\.INFINITE_TIMEOUT_MS|requestTimeoutMillis|socketTimeoutMillis' --glob '**/*ApiClient*.kt'

Repository: OpenHub-Store/GitHub-Store

Length of output: 2541


🏁 Script executed:

sed -n '20,50p' core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 1238


🏁 Script executed:

wc -l core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 155


🏁 Script executed:

cat -n core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 3716


Replace infinite timeouts with finite values for metadata calls.

ForgejoApiClient applies infinite timeouts (HttpTimeoutConfig.INFINITE_TIMEOUT_MS) to both requestTimeoutMillis and socketTimeoutMillis for all API calls. These endpoints fetch repository metadata and releases—small JSON payloads—and infinite timeouts risk leaving connections stuck on half-open TCP sockets, especially in polling scenarios. A finite timeout of 60 seconds balances responsiveness with the typical latency of Forgejo instances.

Proposed fix
         install(HttpTimeout) {
-            requestTimeoutMillis = HttpTimeoutConfig.INFINITE_TIMEOUT_MS
+            requestTimeoutMillis = 60_000
             connectTimeoutMillis = 30_000
-            socketTimeoutMillis = HttpTimeoutConfig.INFINITE_TIMEOUT_MS
+            socketTimeoutMillis = 60_000
         }
📝 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
requestTimeoutMillis = HttpTimeoutConfig.INFINITE_TIMEOUT_MS
connectTimeoutMillis = 30_000
socketTimeoutMillis = HttpTimeoutConfig.INFINITE_TIMEOUT_MS
requestTimeoutMillis = 60_000
connectTimeoutMillis = 30_000
socketTimeoutMillis = 60_000
🤖 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
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt`
around lines 35 - 37, ForgejoApiClient currently sets requestTimeoutMillis and
socketTimeoutMillis to HttpTimeoutConfig.INFINITE_TIMEOUT_MS; change both to a
finite 60_000 (60s) so metadata/release API calls use a 60s timeout while
leaving connectTimeoutMillis at 30_000; update the HttpClient/config block that
sets requestTimeoutMillis and socketTimeoutMillis (the code referencing
HttpTimeoutConfig.INFINITE_TIMEOUT_MS in ForgejoApiClient) to use 60_000
instead.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt (1)

1082-1088: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing sourceHost when fetching release for update breaks Forgejo apps.

updateSingleApp fetches the latest release without passing app.sourceHost, so it always hits the GitHub API even for Forgejo-linked repositories. Updates will fail for any non-GitHub source.

🐛 Proposed fix
                     val latestRelease =
                         try {
                             appsRepository.getLatestRelease(
                                 owner = app.repoOwner,
                                 repo = app.repoName,
                                 includePreReleases = app.includePreReleases,
+                                sourceHost = app.sourceHost,
                             )
                         } catch (e: Exception) {
🤖 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
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`
around lines 1082 - 1088, In updateSingleApp, the call to
appsRepository.getLatestRelease omits the sourceHost, causing non-GitHub sources
to be queried against GitHub; fix by passing sourceHost = app.sourceHost into
the getLatestRelease call (updateSingleApp ->
appsRepository.getLatestRelease(..., sourceHost = app.sourceHost)) so Forgejo
and other hosts are queried correctly.
🧹 Nitpick comments (1)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt (1)

2073-2095: 💤 Low value

Dead code: parseGithubUrl is no longer used.

validateAndLinkRepo now uses RepositoryUrlParser.parse(url) instead. This private function can be removed.

🤖 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
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`
around lines 2073 - 2095, The private function parseGithubUrl is dead code and
should be removed: delete the parseGithubUrl(...) method from AppsViewModel
since validateAndLinkRepo now delegates to RepositoryUrlParser.parse(url);
ensure there are no remaining references to parseGithubUrl in AppsViewModel or
elsewhere and run tests/compile to confirm RepositoryUrlParser.parse is the
single source of truth for repo URL parsing.
🤖 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.

Outside diff comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 1082-1088: In updateSingleApp, the call to
appsRepository.getLatestRelease omits the sourceHost, causing non-GitHub sources
to be queried against GitHub; fix by passing sourceHost = app.sourceHost into
the getLatestRelease call (updateSingleApp ->
appsRepository.getLatestRelease(..., sourceHost = app.sourceHost)) so Forgejo
and other hosts are queried correctly.

---

Nitpick comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 2073-2095: The private function parseGithubUrl is dead code and
should be removed: delete the parseGithubUrl(...) method from AppsViewModel
since validateAndLinkRepo now delegates to RepositoryUrlParser.parse(url);
ensure there are no remaining references to parseGithubUrl in AppsViewModel or
elsewhere and run tests/compile to confirm RepositoryUrlParser.parse is the
single source of truth for repo URL parsing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e30d1a8c-2501-40f7-a5bb-8d4bd3508ef7

📥 Commits

Reviewing files that changed from the base of the PR and between d59e2cc and 4faed5d.

📒 Files selected for processing (21)
  • core/presentation/src/commonMain/composeResources/files/whatsnew/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ar/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/bn/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/es/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/fr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/hi/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/it/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ja/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ko/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/pl/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ru/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/tr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/18.json
  • feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt
  • feature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsRoot.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/CompactAppRow.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/components/SourceChip.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/mappers/InstalledAppMapper.kt
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/model/InstalledAppUi.kt
✅ Files skipped from review due to trivial changes (8)
  • core/presentation/src/commonMain/composeResources/files/whatsnew/bn/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/fr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ru/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ar/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/tr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/es/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/hi/18.json
🚧 Files skipped from review as they are similar to previous changes (6)
  • core/presentation/src/commonMain/composeResources/files/whatsnew/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/pl/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/it/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ja/18.json
  • feature/apps/domain/src/commonMain/kotlin/zed/rainxch/apps/domain/repository/AppsRepository.kt
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ko/18.json

…te timeouts, repoId namespace, broader host detection
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt`:
- Around line 43-45: The close() function currently only cancels the coroutine
scope and leaves cached ForgejoApiClient instances open; update
ForgejoClientRegistry.close() to synchronously close all cached clients before
cancelling the scope by iterating over the cache (the map holding
ForgejoApiClient instances), calling each client's close/shutdown method,
removing/clearing entries, then cancelling the scope; if close() must remain
non-suspending, wrap the client-closing logic in runBlocking (or extract a
suspend helper like suspend fun closeAllClients() and call it via runBlocking)
to ensure clients are closed reliably.
- Around line 27-31: Add explicit resource cleanup: implement a close() function
on ForgejoApiClient that calls the internal HttpClient.close(), then update
ForgejoClientRegistry to call each cached client's close() before discarding
them — replace the mutex.withLock { clients.clear() } invalidation with
mutex.withLock { clients.values.forEach { it.close() }; clients.clear() } and
similarly modify the ForgejoClientRegistry.close() method to close all cached
ForgejoApiClient instances before canceling the registry scope.
🪄 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: 48fc90b7-9e52-44d0-bbfc-fd30e811ff36

📥 Commits

Reviewing files that changed from the base of the PR and between 4faed5d and b5d5f89.

📒 Files selected for processing (6)
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepoIdCodec.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepositoryUrlParser.kt
  • feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt
✅ Files skipped from review due to trivial changes (1)
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepoIdCodec.kt
🚧 Files skipped from review as they are similar to previous changes (3)
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.kt
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt
  • feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt

Comment on lines +27 to +31
.onEach { _ ->
// Invalidate cached clients so the next clientFor()
// rebuilds them against the new proxy config.
mutex.withLock { clients.clear() }
}.launchIn(scope)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if ForgejoApiClient has a close method or holds an HttpClient
ast-grep --pattern $'class ForgejoApiClient($$$) {
  $$$
}'

# Also check for HttpClient usage in ForgejoApiClient
rg -n "HttpClient|\.close\(\)" --type=kt -g '*ForgejoApiClient*'

Repository: OpenHub-Store/GitHub-Store

Length of output: 7609


🏁 Script executed:

# Read the full ForgejoApiClient to check for close/dispose methods
wc -l core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt
cat -n core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 3686


🏁 Script executed:

# Read the full ForgejoClientRegistry to see the context
wc -l core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt
cat -n core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 1925


🏁 Script executed:

# Check if Closeable or AutoCloseable are implemented by ForgejoApiClient
rg "class ForgejoApiClient|Closeable|AutoCloseable" core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoApiClient.kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 94


🏁 Script executed:

# Search for how HttpClient is used elsewhere in the codebase
rg "HttpClient" core/ --type=kt -B2 -A2 | head -100

Repository: OpenHub-Store/GitHub-Store

Length of output: 97


🏁 Script executed:

# Check if HttpClient has a close method in Ktor and if it's commonly called
rg "\.close\(\)|Closeable|AutoCloseable" core/ --type=kt

Repository: OpenHub-Store/GitHub-Store

Length of output: 97


🏁 Script executed:

# Look for any existing close/dispose patterns in the codebase
rg "fun close|suspend fun close" core/ --type=kt -B1 -A3

Repository: OpenHub-Store/GitHub-Store

Length of output: 97


🏁 Script executed:

# Search for HttpClient usage patterns without type filter
rg "HttpClient" core/ -B2 -A2 | head -80

Repository: OpenHub-Store/GitHub-Store

Length of output: 8364


🏁 Script executed:

# Check if HttpClient implements Closeable
rg "close|Closeable" core/ | grep -i "httpclient\|client.close"

Repository: OpenHub-Store/GitHub-Store

Length of output: 784


🌐 Web query:

Ktor HttpClient close() method multiplatform Kotlin 2.3.10

💡 Result:

In Ktor, the HttpClient.close method is used to release resources associated with the client, such as threads, connections, and the CoroutineScope [1][2]. This method is available in the common source set for Kotlin Multiplatform projects [3]. Key points regarding the use of close: 1. Purpose: Calling close initiates a non-blocking shutdown process [2]. It prevents the creation of new requests but allows existing, active requests to complete before fully releasing resources [1]. 2. When to use: You should call close when you no longer need the HttpClient instance [1][2]. For short-lived clients, you can use the use extension function, which automatically calls close after the block of code finishes [1]: val status = HttpClient.use { client -> // Perform requests here } 3. Multiplatform Context: In many long-running mobile applications (Android/iOS), the HttpClient is often treated as a singleton or held for the duration of the application's lifecycle [4]. In such cases, explicit closure may not be strictly necessary if the client lives as long as the application process itself [3]. However, if you are managing the client's lifecycle manually (e.g., in specific features or test suites), you should call close to prevent resource leaks [1][4]. The close method is part of the Ktor client core and functions consistently across supported platforms [5][2].

Citations:


ForgejoApiClient must expose a close() method, and ForgejoClientRegistry must close cached clients before discarding them.

HttpClient holds system resources (threads, connections, scope) that must be explicitly released. Discarding ForgejoApiClient instances without calling close() on their internal HttpClient creates a resource leak. This mirrors the pattern already established in your codebase (TranslationClientProvider, GitHubClientProvider, BackendApiClient all implement proper closure).

ForgejoApiClient needs a close() method that closes the internal HttpClient:

fun close() {
    client.close()
}

Then update ForgejoClientRegistry to close clients before clearing at line 30:

mutex.withLock {
    clients.values.forEach { it.close() }
    clients.clear()
}

Also update the close() method at line 43 to close cached clients before canceling the scope.

🤖 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
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt`
around lines 27 - 31, Add explicit resource cleanup: implement a close()
function on ForgejoApiClient that calls the internal HttpClient.close(), then
update ForgejoClientRegistry to call each cached client's close() before
discarding them — replace the mutex.withLock { clients.clear() } invalidation
with mutex.withLock { clients.values.forEach { it.close() }; clients.clear() }
and similarly modify the ForgejoClientRegistry.close() method to close all
cached ForgejoApiClient instances before canceling the registry scope.

Comment on lines +43 to +45
fun close() {
scope.cancel()
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Close cached clients when the registry is shut down.

close() cancels the coroutine scope but leaves cached ForgejoApiClient instances orphaned. These should be closed alongside scope cancellation.

Proposed fix
-    fun close() {
+    suspend fun close() {
         scope.cancel()
+        mutex.withLock {
+            clients.values.forEach { it.close() }
+            clients.clear()
+        }
     }

If close() must remain non-suspend (e.g., called from a non-coroutine context), use runBlocking or extract a helper:

     fun close() {
         scope.cancel()
+        // Safe since scope is already cancelled; no concurrent modifications
+        clients.values.forEach { it.close() }
+        clients.clear()
     }
📝 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 close() {
scope.cancel()
}
suspend fun close() {
scope.cancel()
mutex.withLock {
clients.values.forEach { it.close() }
clients.clear()
}
}
🤖 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
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ForgejoClientRegistry.kt`
around lines 43 - 45, The close() function currently only cancels the coroutine
scope and leaves cached ForgejoApiClient instances open; update
ForgejoClientRegistry.close() to synchronously close all cached clients before
cancelling the scope by iterating over the cache (the map holding
ForgejoApiClient instances), calling each client's close/shutdown method,
removing/clearing entries, then cancelling the scope; if close() must remain
non-suspending, wrap the client-closing logic in runBlocking (or extract a
suspend helper like suspend fun closeAllClients() and call it via runBlocking)
to ensure clients are closed reliably.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 (1)
feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt (1)

1902-1910: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset validation state before exiting release-fetch errors.

At Line 1909, return@launch in the non-rate-limit catch exits while isValidatingRepo is still true and linkValidationStatus is still set, which can leave the link flow stuck.

Suggested fix
                 val latestRelease =
                     try {
                         appsRepository.getLatestRelease(owner, repo, sourceHost = sourceHost)
                     } catch (e: RateLimitException) {
                         throw e
                     } catch (e: Exception) {
                         logger.debug("Could not fetch release for validation: ${e.message}")
+                        _state.update {
+                            it.copy(
+                                isValidatingRepo = false,
+                                linkValidationStatus = null,
+                                repoValidationError = getString(Res.string.failed_to_link, e.message ?: ""),
+                            )
+                        }
                         return@launch
                     }
🤖 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
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`
around lines 1902 - 1910, In the non-rate-limit catch block around
appsRepository.getLatestRelease (inside the coroutine launched in
AppsViewModel), before returning@launch ensure you reset the validation state by
setting isValidatingRepo = false and clearing or updating linkValidationStatus
to the appropriate idle/error value so the link flow isn't left stuck; update
the catch for Exception to perform these state resets (and then return@launch).
🤖 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 `@core/presentation/src/commonMain/composeResources/values-es/strings-es.xml`:
- Line 921: The string resource custom_forges_count_label currently always uses
the plural form ("%1$d añadidas"); change it to handle singular vs plural by
replacing it with an Android plurals resource named custom_forges_count_label
and provide at least quantity="one" ("%1$d añadida") and quantity="other" ("%1$d
añadidas"), or alternatively use a neutral form that works for all counts;
update any callers (e.g., code that calls getQuantityString on
custom_forges_count_label) to use getQuantityString with the actual count.

In `@core/presentation/src/commonMain/composeResources/values-fr/strings-fr.xml`:
- Line 922: The string resource custom_forges_count_label is currently
plural-only ("%1$d ajoutées") and will be wrong for count == 1; replace this
with a proper Android/i18n plural resource (plurals) or a grammatically neutral
form. Create a plurals entry (e.g., name="custom_forges_count") with one and
other forms for singular and plural in values-fr/strings-fr.xml and update any
references to use getQuantityString/getPluralResource for custom_forges_count so
the app displays the correct French form for 1 vs. multiple.

In `@core/presentation/src/commonMain/composeResources/values-it/strings-it.xml`:
- Line 961: The string resource custom_forges_count_label currently uses the
plural-only value "%1$d aggiunte"; replace it with proper pluralization by
creating a plurals resource (e.g., name it custom_forges_count) with at least
quantity="one" -> "1 aggiunta" and quantity="other" -> "%d aggiunte", then
update any places that use custom_forges_count_label to call the quantity-aware
API (getQuantityString/getQuantity) with the actual count so the singular form
is used for 1 and the plural for other counts.

In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 1036-1039: The current use of runCatching in TweaksViewModel
(inside viewModelScope.launch around tweaksRepository.addCustomForgeHost and
removeCustomForgeHost) drops errors and clears UI state unconditionally; change
each runCatching to handle success and failure (use .onSuccess/.onFailure or
.fold) so that on success you call _state.update to clear
customForgeDraft/customForgeError (for add) or remove the host from the list
(for remove), and on failure you do not clear the draft but instead set
customForgeError (e.g., exception.message) so persistence failures are surfaced
to the UI; keep all updates using the same _state.update call sites referenced
in this diff.

---

Outside diff comments:
In
`@feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt`:
- Around line 1902-1910: In the non-rate-limit catch block around
appsRepository.getLatestRelease (inside the coroutine launched in
AppsViewModel), before returning@launch ensure you reset the validation state by
setting isValidatingRepo = false and clearing or updating linkValidationStatus
to the appropriate idle/error value so the link flow isn't left stuck; update
the catch for Exception to perform these state resets (and then return@launch).
🪄 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: 978f6e2b-ba72-4e49-b66c-fe8e7cc6f3af

📥 Commits

Reviewing files that changed from the base of the PR and between b5d5f89 and e412761.

📒 Files selected for processing (36)
  • core/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/TweaksRepositoryImpl.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/repository/TweaksRepository.kt
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepositoryUrlParser.kt
  • core/presentation/src/commonMain/composeResources/files/whatsnew/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ar/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/bn/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/es/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/fr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/hi/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/it/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ja/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ko/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/pl/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ru/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/tr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/18.json
  • core/presentation/src/commonMain/composeResources/values-ar/strings-ar.xml
  • core/presentation/src/commonMain/composeResources/values-bn/strings-bn.xml
  • core/presentation/src/commonMain/composeResources/values-es/strings-es.xml
  • core/presentation/src/commonMain/composeResources/values-fr/strings-fr.xml
  • core/presentation/src/commonMain/composeResources/values-hi/strings-hi.xml
  • core/presentation/src/commonMain/composeResources/values-it/strings-it.xml
  • core/presentation/src/commonMain/composeResources/values-ja/strings-ja.xml
  • core/presentation/src/commonMain/composeResources/values-ko/strings-ko.xml
  • core/presentation/src/commonMain/composeResources/values-pl/strings-pl.xml
  • core/presentation/src/commonMain/composeResources/values-ru/strings-ru.xml
  • core/presentation/src/commonMain/composeResources/values-tr/strings-tr.xml
  • core/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xml
  • core/presentation/src/commonMain/composeResources/values/strings.xml
  • feature/apps/presentation/src/commonMain/kotlin/zed/rainxch/apps/presentation/AppsViewModel.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksRoot.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksState.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/CustomForgesDialog.kt
  • feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Network.kt
✅ Files skipped from review due to trivial changes (11)
  • core/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xml
  • core/presentation/src/commonMain/composeResources/values-ko/strings-ko.xml
  • core/presentation/src/commonMain/composeResources/files/whatsnew/bn/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ko/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/tr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/pl/18.json
  • core/presentation/src/commonMain/composeResources/values/strings.xml
  • core/presentation/src/commonMain/composeResources/files/whatsnew/fr/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ru/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ar/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/es/18.json
🚧 Files skipped from review as they are similar to previous changes (6)
  • core/presentation/src/commonMain/composeResources/files/whatsnew/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/hi/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/18.json
  • core/presentation/src/commonMain/composeResources/files/whatsnew/it/18.json
  • core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/RepositoryUrlParser.kt
  • core/presentation/src/commonMain/composeResources/files/whatsnew/ja/18.json

<!-- Tweaks - Custom forges -->
<string name="custom_forges_entry_label">Forjas personalizadas</string>
<string name="custom_forges_entry_subtitle">Añade hosts Forgejo / Gitea que quieras que GHS reconozca</string>
<string name="custom_forges_count_label">%1$d añadidas</string>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Adjust count label to handle singular correctly.

%1$d añadidas assumes plural and reads wrong for 1. Please use neutral wording (or proper pluralization) so the label is correct for all counts.

🤖 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 `@core/presentation/src/commonMain/composeResources/values-es/strings-es.xml`
at line 921, The string resource custom_forges_count_label currently always uses
the plural form ("%1$d añadidas"); change it to handle singular vs plural by
replacing it with an Android plurals resource named custom_forges_count_label
and provide at least quantity="one" ("%1$d añadida") and quantity="other" ("%1$d
añadidas"), or alternatively use a neutral form that works for all counts;
update any callers (e.g., code that calls getQuantityString on
custom_forges_count_label) to use getQuantityString with the actual count.

<!-- Tweaks - Custom forges -->
<string name="custom_forges_entry_label">Forges personnalisées</string>
<string name="custom_forges_entry_subtitle">Ajoute des hôtes Forgejo / Gitea que tu veux que GHS reconnaisse</string>
<string name="custom_forges_count_label">%1$d ajoutées</string>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Make the count label valid for singular too.

%1$d ajoutées is plural-only and will be incorrect when the count is 1. Please use neutral wording (or pluralization) to keep this grammatically correct.

🤖 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 `@core/presentation/src/commonMain/composeResources/values-fr/strings-fr.xml`
at line 922, The string resource custom_forges_count_label is currently
plural-only ("%1$d ajoutées") and will be wrong for count == 1; replace this
with a proper Android/i18n plural resource (plurals) or a grammatically neutral
form. Create a plurals entry (e.g., name="custom_forges_count") with one and
other forms for singular and plural in values-fr/strings-fr.xml and update any
references to use getQuantityString/getPluralResource for custom_forges_count so
the app displays the correct French form for 1 vs. multiple.

<!-- Tweaks - Custom forges -->
<string name="custom_forges_entry_label">Forge personalizzate</string>
<string name="custom_forges_entry_subtitle">Aggiungi host Forgejo / Gitea che vuoi che GHS riconosca</string>
<string name="custom_forges_count_label">%1$d aggiunte</string>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix singular/plural count wording for custom forges.

%1$d aggiunte is plural-only and will read incorrectly when the count is 1. Please switch to a grammatically neutral label (or pluralized resources) for correct UX at all counts.

🤖 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 `@core/presentation/src/commonMain/composeResources/values-it/strings-it.xml`
at line 961, The string resource custom_forges_count_label currently uses the
plural-only value "%1$d aggiunte"; replace it with proper pluralization by
creating a plurals resource (e.g., name it custom_forges_count) with at least
quantity="one" -> "1 aggiunta" and quantity="other" -> "%d aggiunte", then
update any places that use custom_forges_count_label to call the quantity-aware
API (getQuantityString/getQuantity) with the actual count so the singular form
is used for 1 and the plural for other counts.

Comment on lines +1036 to +1039
viewModelScope.launch {
runCatching { tweaksRepository.addCustomForgeHost(raw) }
_state.update { it.copy(customForgeDraft = "", customForgeError = null) }
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle add/remove persistence failures explicitly.

At Line 1037 and Line 1044 the runCatching results are dropped. Add currently clears the draft at Line 1038 even if persistence fails, and remove failures are fully silent. Please branch success/failure and only clear UI state on success.

Suggested fix
             TweaksAction.OnAddCustomForge -> {
                 val raw = _state.value.customForgeDraft
@@
                 viewModelScope.launch {
-                    runCatching { tweaksRepository.addCustomForgeHost(raw) }
-                    _state.update { it.copy(customForgeDraft = "", customForgeError = null) }
+                    runCatching { tweaksRepository.addCustomForgeHost(raw) }
+                        .onSuccess {
+                            _state.update { it.copy(customForgeDraft = "", customForgeError = null) }
+                        }
+                        .onFailure { error ->
+                            _state.update {
+                                it.copy(customForgeError = error.message ?: "Failed to save host")
+                            }
+                        }
                 }
             }
@@
             is TweaksAction.OnRemoveCustomForge -> {
                 viewModelScope.launch {
-                    runCatching { tweaksRepository.removeCustomForgeHost(action.host) }
+                    runCatching { tweaksRepository.removeCustomForgeHost(action.host) }
+                        .onFailure { error ->
+                            _state.update {
+                                it.copy(customForgeError = error.message ?: "Failed to remove host")
+                            }
+                        }
                 }
             }

Also applies to: 1043-1045

🤖 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
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`
around lines 1036 - 1039, The current use of runCatching in TweaksViewModel
(inside viewModelScope.launch around tweaksRepository.addCustomForgeHost and
removeCustomForgeHost) drops errors and clears UI state unconditionally; change
each runCatching to handle success and failure (use .onSuccess/.onFailure or
.fold) so that on success you call _state.update to clear
customForgeDraft/customForgeError (for add) or remove the host from the list
(for remove), and on failure you do not clear the draft but instead set
customForgeError (e.g., exception.message) so persistence failures are surfaced
to the UI; keep all updates using the same _state.update call sites referenced
in this diff.

Comment on lines +568 to +573
val releases = client.getReleases(owner, repo, perPage = 10).getOrNull()
?: return null
releases
.asSequence()
.filter { it.draft != true }
.filter { includePreReleases || it.prerelease != true }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Pre-release flag check inconsistent with isEffectivelyPreRelease()

fetchForgejoLatestRelease filters only on it.prerelease != true, while InstalledAppsRepositoryImpl.fetchForgejoReleaseWindow converts to the domain model and then calls isEffectivelyPreRelease(), which additionally inspects the tag name and release name via VersionMath.isPreReleaseTag. A Forgejo release tagged v2.0.0-rc.1 but published with the prerelease field set to false (e.g., a maintainer who forgot to tick the flag) would be excluded by the update-check window but returned as the "latest" here, surfacing a pre-release build to a user who opted out.

Fix in Claude Code

Comment on lines +3 to +9
object RepoIdCodec {
fun encode(host: String?, rawId: Long): Long {
if (host.isNullOrBlank()) return rawId
val hostBits = (host.lowercase().hashCode().toLong() and 0x7FFFL) shl 48
val idBits = rawId and 0x0000FFFFFFFFFFFFL
return -(hostBits or idBits)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 RepoIdCodec reserves only 15 bits for the host fingerprint (0x7FFF = 32 768 buckets). Two different Forgejo instances can produce the same 15-bit truncation of hashCode() — any two hostnames whose Java String.hashCode() values agree in bits 0–14 will collide. When the same numeric repo ID also appears on both instances, the encoded repoId is identical. Any DAO query that looks up by repoId alone (getAppByRepoId, isAppInstalled, etc.) will silently return the wrong app, the same class of data-corruption the codec was introduced to prevent between GitHub and Forgejo.

Widening to 24 bits (0xFFFFFF, 16 million buckets) and adjusting the ID bits to 39 bits makes the collision probability negligible while keeping the result within Long range.

Suggested change
object RepoIdCodec {
fun encode(host: String?, rawId: Long): Long {
if (host.isNullOrBlank()) return rawId
val hostBits = (host.lowercase().hashCode().toLong() and 0x7FFFL) shl 48
val idBits = rawId and 0x0000FFFFFFFFFFFFL
return -(hostBits or idBits)
}
object RepoIdCodec {
fun encode(host: String?, rawId: Long): Long {
if (host.isNullOrBlank()) return rawId
val hostBits = (host.lowercase().hashCode().toLong() and 0xFFFFFFL) shl 39
val idBits = rawId and 0x7FFFFFFFFFL
return -(hostBits or idBits)
}

Fix in Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant