Repository navigation
Audit fixes: stages 1-2 + offline reading (Critical/High core closed) - #10
Merged
Merged
Conversation
…021, G-004, G-005) G-001 — OAuth Device Flow now requests the `gist` scope; without it POST /gists answered 404, so the "New gist" screen failed for every user signed in through Device Flow. PAT hints updated in EN and RU. G-002 — the access token (scope `repo` = write access to private repos) was stored as plaintext in DataStore. It is now encrypted with an AES-256/GCM key that never leaves the Android Keystore, legacy plaintext tokens are migrated in place on first run, and a lost/invalidated key degrades to a signed-out session instead of a crash. Keystore work runs on Dispatchers.IO. G-003 — PR changed-files are paginated through the Link header; a PR with more than 100 files used to show a silently truncated diff. G-021 — notification routing is type-aware (NotificationRouting): only Issue and PullRequest threads open an issue screen; Release/Discussion/Commit/ CheckSuite land on the repository instead of a non-existent issue (404). G-004 — the unread badge reads one shared count from TokenStore, computed over all notification pages, and is updated on mark-read / mark-all-read instead of going stale. G-005 — 429 and secondary rate limits (Retry-After) map to the rate-limit key instead of a generic HTTP error. Blocked in this environment: the CI parts of G-011 (lintDebug step) and G-042 (client id from a secret) touch .github/workflows/build.yml, which the GitHub App of this session is not allowed to modify (no `workflows` permission), and the Actions secrets API answers 403. Both are documented in the PR instead. Tests: NotificationRoutingTest (10 cases), TokenCipherTest (5 cases). Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
… untouched Detekt failed on the previous revision (exit code 1) and CI logs are not readable from this environment. Two hypotheses, both removed here: - TokenCipher caught the generic `Exception`, which TooGenericExceptionCaught flags by default, and relied on a detekt.yml exclude that I could not verify locally (no Kotlin compiler or Maven access in the sandbox). It now catches GeneralSecurityException, which already covers KeyStoreException, UnrecoverableKeyException and every cipher failure, so the existing config is correct as-is and detekt.yml is reverted. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…urns
Detekt still failed after the first fixup. The remaining finding in the new
code is TokenCipher.generateKey(store): KeyGenerator binds the key to
AndroidKeyStore on its own, so the parameter was unused (UnusedParameter), and
encrypt()/keyOrNull() used non-local returns out of `?: run {}` and `try`
expressions (NonLocalReturnControl candidates). Both are rewritten as plain
control flow; the file is otherwise unchanged.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
G-010 — the screen a developer opens a GitHub client for: issues assigned to me, issues I created, and pull requests waiting for my review. - SearchQueries.myWorkQuery() builds the three search queries (assignee:@me / author:@me / review-requested:@me, state:open), sorted by recent activity; unit-tested because the qualifier syntax is an API contract. - MyWorkViewModel keeps one list + pagination cursor per tab, so switching tabs never loses state and never re-queries; a failed refresh keeps the previous list, a failed "load more" stays retryable. - MyWorkScreen: FilterChips, per-tab empty states, pull-to-refresh, footer. - Reached from the Home app bar; rows open the issue/PR in place, which needed `repository_url` on the Issue DTO (search results only) — exposed as Issue.repositoryFullName via GitHubUrls.parseApiRepoUrl. G-014 — merging a PR is now possible in the app, which was the last blocker for the review workflow: - PUT /pulls/{n}/merge with merge/squash/rebase, `sha` pinned to the head commit the user reviewed, so a branch that moved is rejected instead of merging unseen commits. - Typed failures: 409 -> MergeConflictException ("branch changed, reload"), 405/422 -> MergeNotAllowedException ("checks/review/branch protection"), everything else -> the existing error mapping. - MergeDialog shows mergeable_state before the tap (conflicts / behind / blocked / ready) and disables the confirm button on a known conflict. Strings in EN and RU; parity check passes. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…ab-safe
Two real defects in the previous commit, both caught by the unit-test step:
- IssuesRepository.merge() compared `when {}` branches against Int while the
subject was Response<Unit>: `409 ->` never compiled. Now `response.code()` is
compared, with an explicit Result<Unit> return type.
- MyWorkViewModel rebuilt its state from the default MyWorkUi() after the
first fetch, so a tab picked while a request was in flight was silently
reset to "assigned" and results could land in the wrong tab. Rewritten to
assemble state from a per-tab map keyed by the tab actually loaded, with
`refreshing`/`loadedOnce` per tab, an emitted error only for the first
failed fetch, and no lost pull-to-refresh spinner.
Adds a GitHubUrls test for the api `repository_url` that My work rows rely on.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
The merge confirm button shows CircularWavyProgressIndicator while the request is in flight; that composable is @ExperimentalMaterial3ExpressiveApi, and the dialog was declared without @OptIn, unlike every other call site in the app. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
merge() wraps its request in safeCall, which lives in data.util; the file only imported LinkHeaderParser from that package, so the module did not compile (detekt passes on it — the unit-test step is what caught it). Verified the rest of the Stage 2 code for the same class of missing cross-package import. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
oustery
force-pushed
the
arena/cc336576-gityou
branch
from
November 9, 2026 17:16
33cc85b to
ba2fb61
Compare
issue editing (G-008), accessibility fixes (G-006) G-007 — github.com links inside rendered markdown (issue bodies, comments, READMEs, release notes) used to open the browser, throwing the reader out of the conversation they were following: - GitHubUrls.routeLink() classifies a link as issue/PR thread, repository or profile; deeper paths (/blob/, /tree/, /releases) and foreign hosts return null and stay in the browser, because opening a different screen would silently drop the user's destination. - MarkdownLinkTouchListener resolves the URLSpan under the finger and asks the host screen first; anything declined keeps LinkMovementMethod's default, so external links, selection and scrolling are unchanged. - One router is installed around the NavHost, so every markdown surface behaves the same. G-012 — the app claimed no links and accepted no shares: - App Links intent-filter for https://github.com hosted links (autoVerify), ACTION_SEND for text/plain, handled by AppViewModel so an inbound link survives sign-in and rotation, then drained by the UI once the graph exists. - DeepLinkRouting maps links to routes and extracts the first GitHub URL out of share text (share sheets prepend their own blurb). G-008 — an issue could be closed or relabelled but never edited: - PATCH /issues/{n} through IssuesRepository.updateIssue(); the dialog pre-fills the current title/body so editing happens in place. - "Copy link" puts the issue URL on the clipboard (htmlUrl now part of the detail model). G-006 — accessibility: - EyebrowLabel is a semantics heading, so TalkBack can jump between sections. - The pull-request "files changed" button announces the real file count instead of a hardcoded 0. Unit tests cover link classification, deep-link routes and share extraction. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Two compile errors from the previous commit: - CompositionLocalProvider was placed inside the NavHost *builder* block, which is a plain lambda rather than a composable scope, and the NavHost call itself was lost while restructuring. The provider now wraps NavHost properly. - IssueDetailScreen calls scope.launch for the clipboard snackbar without importing kotlinx.coroutines.launch. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
In a repository with hundreds of issues the list showed every issue of one state, newest first, with no way to narrow it down — the tab was effectively unusable for real triage. - GitHubApi.issues() takes labels/assignee/creator/sort/direction, so the filtering happens on the server instead of over a single loaded page. - ReposRepository.IssueQuery bundles those filters; `hasFilters` distinguishes "the user filtered something out" from the default view, which the empty state needs to say something different. - RepoViewModel: toggleIssueLabel (AND semantics, as the API defines it), toggleAssignedToMe (@me), setIssueSort (created / most commented) and an in-list search over the loaded page ("no results" without a request). - RepoScreen: filter chips, label chips from the repository palette (loaded once), search field with a clear button, and a "clear filters" action. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
A token with write access to every repository the user can push to was stored on the device with nothing between a lost phone and the repositories. - AppSettings gains biometricLock + lockTimeoutMinutes; Settings gained a Security section with the switch and the re-lock delay (immediately / 1 / 5 / 15 min). - AppLockScreen verifies the device credential through KeyguardManager.createConfirmDeviceCredentialIntent: the platform checks the secret, the app never stores or compares one. A device without any screen lock says so and lets the user in instead of bricking the client. - Re-locking is lifecycle-driven: onStop records the time unless the activity is only reconfiguring, onStart re-arms the lock once the configured delay has passed, so rotation never locks mid-use but a real background trip does. - Settings offers "Revoke GitHub access": signing out only forgets the token locally, and revocation itself needs the OAuth client secret that a public client must not embed, so the user is sent to the official page. Unit tests (G-011): GitHubApiNetworkTest runs the real Retrofit stack against MockWebServer and pins JSON parsing, the 403/429 rate-limit mapping, offline mapping and the PR merge request body/status. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
detekt's TopLevelPropertyNaming rejects SCREAMING_CASE for a non-const val (maxIssues is 0, so it failed the build). Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
The Settings edition dropped the `}` that closes UpdateDialog (my patch replaced it with the new file-level declarations), so the module did not parse: detekt reported it and nothing past static analysis ran. Restores the brace, keeps the lock-timeout list as a lowerCamelCase property (TopLevelPropertyNaming) and brings back the MockWebServer tests that were temporarily removed while bisecting. Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Every screen turned into a full-screen error the moment connectivity dropped,
even for a repository the user had just been reading — the metro/lift case,
which is a large share of mobile sessions.
- OfflineCache configures a 50 MB OkHttp disk cache on the authenticated
client and installs a fallback interceptor: when the request cannot reach
the network, it retries `only-if-cached`/`max-stale` and serves the stored
body. GitHub already sends `Cache-Control: private, max-age=60` plus ETags,
so revalidation stays cheap; errors are never cached, because serving a
stored 404 as "offline data" would be worse than the error.
- A plain HTTP cache was chosen over a Room mirror: it makes every screen work
offline at once without duplicating the DTO model, and keeps privacy intact
(the cache lives in the app's private cache dir and is wiped with the app).
- OfflineState carries the one bit the UI needs ("this came from the cache"),
set by the interceptor and cleared by the next successful response.
- GitYouApp draws a slim banner above the content while data is stale, so the
user never mistakes saved content for live content.
Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
# Conflicts:
# app/src/main/java/com/gityou/app/ui/AppViewModel.kt
# app/src/main/java/com/gityou/app/ui/GitYouApp.kt
oustery
changed the title
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
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.
Что это
Реализация плана из аудита
docs/AUDIT_COMPLETENESS.md: закрыты все Critical-находки и ядро High — авторизация, безопасность токена, навигация, работа с issue/PR, офлайн.Этап 1 — Critical
gistв OAuth scope + подсказка про PAT: создание гиста больше не падает с 404 у новых пользователейTokenCipher); плейнтекст старых сборок мигрируется на местеpullFilesобходит Link-header: диффы PR на 100+ файлов больше не теряют хвостRetry-Afterраспознаются как rate limit, а не «HTTP ошибка»Этап 2 — High
/blob/,/releasesи чужие хосты остаются в браузере.github.com+ приём «Поделиться в GitYou» (ссылка переживает вход и поворот экрана).shaпривязан к просмотренному коммиту, 409/405/422 разведены в понятные сообщения.Этап 3 (начат)
only-if-cachedи баннер «показаны сохранённые данные». HTTP-кэш выбран вместо Room, чтобы покрыть все экраны сразу и не дублировать DTO.Тесты
Добавлены:
TokenCipherTest,NotificationRoutingTest,SearchQueriesTest,GitHubUrlsTest(включая маршрутизацию ссылок),DeepLinkRoutingTest,GitHubApiNetworkTest(реальный Retrofit против MockWebServer: парсинг, 403/429, офлайн, тело запроса мержа).Что осталось за рамками PR
./gradlew lintDebugподготовлен, но правку.github/workflows/build.ymlблокирует GitHub App (нет scopeworkflows) — нужен человек с правами на репозиторий.Проверка
CI
Build: detekt → unit-тесты → сборка per-ABI APK. Последний прогон на ветке — зелёный.