Review and freeze TASK-004/005/006; fix 409/422 typed failures

Independent read-only review of 8a47d30/bbbff77/16c46a5 per
handoff/claude-master-handoff.md's review-and-freeze step. Verified
build/test/lint independently rather than trusting recorded evidence.

Fixed P2: requireSuccess() collapsed HTTP 409/422 into the generic
InvalidResponse, which would have blocked TASK-007's planned
conflict/unprocessable error states. Added ApiFailure.Conflict and
ApiFailure.Unprocessable with test coverage.

Recorded a P3 (CPU-bound work on the Main dispatcher in
CodeBrowserViewModel) for TASK-016 rather than fixing it now. Closed
TASK-005/006's mutual contract-freeze checkboxes. No P0/P1 found.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

app/src/main/java/com/ubisam/ubgitea/core/network/HttpTransport.kt

@@ -81,6 +81,8 @@ sealed class ApiFailure(message: String) : Exception(message) {
     class AuthenticationExpired : ApiFailure("Authentication expired")
     class PermissionDenied : ApiFailure("Permission denied")
     class NotFound : ApiFailure("Resource not found")
+    class Conflict : ApiFailure("Conflict")
+    class Unprocessable : ApiFailure("Unprocessable request")
     class RateLimited : ApiFailure("Rate limited")
     class Network(cause: IOException) : ApiFailure("Network unavailable") {
         init {
@@ -96,6 +98,8 @@ fun HttpResponse.requireSuccess(): HttpResponse = when (statusCode) {
     401 -> throw ApiFailure.AuthenticationExpired()
     403 -> throw ApiFailure.PermissionDenied()
     404 -> throw ApiFailure.NotFound()
+    409 -> throw ApiFailure.Conflict()
+    422 -> throw ApiFailure.Unprocessable()
     429 -> throw ApiFailure.RateLimited()
     in 500..599 -> throw ApiFailure.Server()
     else -> throw ApiFailure.InvalidResponse()

app/src/test/java/com/ubisam/ubgitea/data/repositories/GiteaRepositoryBrowserApiTest.kt

@@ -140,10 +140,16 @@ class GiteaRepositoryBrowserApiTest {
     }
 
     @Test
-    fun `http status mapping distinguishes not found and rate limited`() {
+    fun `http status mapping distinguishes not found conflict unprocessable and rate limited`() {
         assertThrows(ApiFailure.NotFound::class.java) {
             HttpResponse(404, "", emptyMap()).requireSuccess()
         }
+        assertThrows(ApiFailure.Conflict::class.java) {
+            HttpResponse(409, "", emptyMap()).requireSuccess()
+        }
+        assertThrows(ApiFailure.Unprocessable::class.java) {
+            HttpResponse(422, "", emptyMap()).requireSuccess()
+        }
         assertThrows(ApiFailure.RateLimited::class.java) {
             HttpResponse(429, "", emptyMap()).requireSuccess()
         }

docs/development-log.md

@@ -149,3 +149,18 @@ This is the append-only implementation and handoff record shared by Codex and Cl
 - Known limitations: Physical-device interaction has not yet been rerun. TASK-005's reviewed contract-freeze criterion remains open pending later independent review.
 - Next recommended task: TASK-009 immutable history/change/diff API contracts, followed by TASK-007 read-only history and change UI.
 - Next owner: Codex for the continuous serial stream.
+
+## 2026-08-26 — TASK-004/005/006 independent review and freeze
+
+- Agent and role: Claude, independent reviewer per `handoff/claude-master-handoff.md`'s "review and freeze" first action; human product owner authorized Claude as incoming primary implementer with handback to Codex kept possible at any time.
+- Commit/diff: reviewed `8a47d30` (TASK-004), `bbbff77` (TASK-005), `16c46a5` (TASK-006) read-only, then fixed one finding on branch `claude/review-005-006-freeze` from `16c46a5`.
+- Review scope and method: Independently reran `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` (did not trust the recorded evidence alone) and read `GiteaRepositoryBrowserApi.kt`, `HttpTransport.kt`, `RepositoryListViewModel.kt`, `RepositoryReferencePresenter.kt`, `CodeBrowserViewModel.kt`, `CodeBrowserNavigation.kt`, and `MainActivity.kt` against the master handoff's checklist: races/cancellation, root back, rotation restoration, URL encoding, immutable snapshots, page overflow, and 404/409/422 mapping. TASK-009's commit was not yet available (still uncommitted upstream) and was excluded from this pass per human product owner direction.
+- Findings:
+  - P2 (fixed): `core/network/HttpTransport.kt` `requireSuccess()` mapped 401/403/404/429/5xx but let 409 and 422 fall through to the generic `ApiFailure.InvalidResponse`, which would have blocked TASK-007's planned `conflict`/`unprocessable` UDF states. Added `ApiFailure.Conflict`/`ApiFailure.Unprocessable` and extended `GiteaRepositoryBrowserApiTest`'s status-mapping test.
+  - P3 (recorded, not fixed): `CodeBrowserViewModel` runs JSON parsing, Base64 decoding, directory sort/dedupe, and the TASK-006A classifier on `viewModelScope`'s Main dispatcher after the network suspend point resumes. Not blocking; recommended for TASK-016 (release hardening) given its low-end-device and large-content scope.
+  - Everything else checked (races/cancellation, root back via `BackHandler` in both screens, immutable snapshot wrapping, page-overflow bounds, URL component encoding) verified correct with no finding.
+- Checks and results after the fix: `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` BUILD SUCCESSFUL, 102 unit tests across 23 suites, 0 failures/errors, 0 lint errors; `git diff --check` clean; sensitive-pattern scan clean (one match was a test asserting `client_secret` absence, not a leak).
+- Task evidence updated: TASK-004, TASK-005, and TASK-006 each record this closure. TASK-005's "reviewed completion commit is TASK-006's contract-freeze base" and TASK-006's "evidence records the exact reviewed TASK-005 contract-freeze commit" checkboxes are both closed. TASK-009's corresponding freeze checkbox remains open pending its own commit and review.
+- Known limitations: No physical device was available, so device-interaction checks named in TASK-005/006 remain deferred, unchanged from before this review.
+- Next recommended task: TASK-007 (history and change UI), per the ordered backlog in `handoff/claude-master-handoff.md`, once TASK-009 lands and is reviewed; TASK-010 (work-item API contracts) ownership is transferred to Claude per the same handoff.
+- Next owner: Claude, continuing the primary implementer role with handback to Codex available at any time by flipping a task's Owner field back and picking up from its last recorded commit.

tasks/004-gitea-api-client.md

@@ -1,6 +1,6 @@
 # TASK-004: Implement the read-only Gitea API client
 
-Status: implemented and verified — independent review deferred while Codex owns the serial workstream
+Status: implemented and independently reviewed by Claude; one P2 fixed (409/422 typed failures)
 Owner: Codex
 Reviewer: Claude
 
@@ -45,3 +45,12 @@ Connect the repository browsing domain to authenticated, paginated Gitea API cal
 - Added immutable `TagSummary` and tag-list domain/use-case contract.
 - Focused adapter tests cover mapping, pagination, component encoding, authenticated headers, strict file decoding, malformed responses, 404, 429, and local HTTP transport behavior.
 - Full debug build, unit tests, lint, diff check, and sensitive-pattern scan are recorded in `docs/development-log.md`.
+
+## Claude independent review closure
+
+Reviewed read-only at commit `8a47d30` (audited together with TASK-005 `bbbff77` and TASK-006 `16c46a5` per `handoff/claude-master-handoff.md`'s review-and-freeze instruction). Independently reran `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` at `16c46a5`: BUILD SUCCESSFUL, 102 unit tests across 23 suites, 0 failures/errors, 0 lint errors; `git diff --check` clean; sensitive-pattern scan clean (one match was the test itself asserting `client_secret` is absent from the authorization URI).
+
+- P2 (fixed): `HttpTransport.kt:94-102` `requireSuccess()` mapped only 401/403/404/429/5xx to typed `ApiFailure`; HTTP 409 and 422 fell through to the generic `InvalidResponse`, losing the distinction the review checklist and the upcoming TASK-007 UDF error states (`conflict`/`unprocessable`) require. Fixed by adding `ApiFailure.Conflict` (409) and `ApiFailure.Unprocessable` (422) and extending `GiteaRepositoryBrowserApiTest`'s status-mapping test to cover both. Closure: full gates rerun and passing (above); TASK-007 can now type its conflict/unprocessable states directly from `core/network`.
+- URL encoding (verified, no finding): owner/name/path segments use `HttpUrl.Builder.addPathSegment` individually (`repoUrl`, `RepositoryPath.segments()`); `ref` and `path`-filter values use `addQueryParameter`. Confirmed by `GiteaRepositoryBrowserApiTest`'s encoding assertions on repository names containing spaces/`+`/`#` and refs containing `+`/`/`.
+- Pagination/page overflow (verified, no finding): `listTree`'s local pagination uses `Long` arithmetic before coercing to `Int`, so no overflow on large `page` values; `Int.MAX_VALUE` page numbers are rejected via `safeNextPage()` instead of wrapping.
+- No P0/P1 found. No other P2/P3 raised against this task's scope.

tasks/005-repository-branch-ui.md

@@ -1,6 +1,6 @@
 # TASK-005: Browse repositories and branches
 
-Status: implemented and locally verified — independent review and device check deferred
+Status: implemented and independently reviewed by Claude; no P0/P1 findings; device check deferred
 Owner: Codex
 Reviewer: Claude
 
@@ -28,7 +28,7 @@ A signed-in user can browse every repository and branch visible to their Gitea a
 - [x] Retry and pagination cannot create duplicate items or overlapping requests.
 - [x] Presentation and ViewModel tests cover primary states and selection behavior.
 - [x] Resource names use the `repositories_` prefix.
-- [ ] The reviewed completion commit is recorded as TASK-006's contract-freeze base.
+- [x] The reviewed completion commit is recorded as TASK-006's contract-freeze base.
 
 ## Evidence
 
@@ -38,3 +38,14 @@ A signed-in user can browse every repository and branch visible to their Gitea a
 - Repository-wide `assembleDebug testDebugUnitTest lintDebug` passed: 96 unit tests across 21 suites, zero failures/errors.
 - `git diff --check` passed. Focused sensitive-pattern inspection found no token, client secret, password, private key, or credential material.
 - The independent reviewer and physical-device interaction check are deferred while Codex owns the serial implementation stream. TASK-006 may be developed on the local contract, but its eventual integration review must include this commit before the contract-freeze checkbox is closed.
+
+## Claude independent review closure
+
+Reviewed read-only at commit `bbbff77` per `handoff/claude-master-handoff.md`. Independently reran `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` at `16c46a5` (which includes this commit): BUILD SUCCESSFUL, 102 tests / 23 suites, 0 failures; `git diff --check` clean.
+
+- Races/cancellation (verified, no finding): `selectRepository`, `refreshReferences`, and `retry` all route through `startOperation`, which cancels the prior `Job` before launching the next one, so rapid repository/branch/tag switches cannot overlap.
+- Immutable snapshots (verified, no finding): `RepositoryListUiState.Content` and `ReferencesContent` wrap their lists in `Collections.unmodifiableList`; `backToRepositories()` restores the stored `repositoryContent` snapshot rather than reloading, and the restored snapshot cannot be mutated by later reference-loading state.
+- Page overflow (verified, no finding): `loadAllPages` bounds branch/tag pagination to `MAX_REFERENCE_PAGES` (100) and throws a typed failure instead of looping forever if a server never reports the end of a list.
+- No P0/P1 found. The 409/422 typed-failure gap recorded under TASK-004 was the only cross-cutting finding and is now fixed there.
+
+Device interaction remains deferred (no physical device available in this environment); this is an existing known limitation, not a new finding.

tasks/006-code-browser.md

@@ -1,8 +1,8 @@
 # TASK-006: Browse trees and source code by ref
 
-Status: implemented and locally verified — independent review/device check deferred
+Status: implemented and independently reviewed by Claude; no P0/P1 findings; device check deferred
 Owner: Codex
-Reviewer: Claude (deferred until independent review is re-enabled)
+Reviewer: Claude
 
 ## Outcome
 
@@ -29,7 +29,7 @@ Shared domain, data, Gradle, and manifest contracts remain unchanged. Codex temp
 - [x] Loading, empty, offline, authentication-expired, not-found, and server-error states are localized and accessible.
 - [x] Tests cover ref preservation, path navigation, content classification, and size limits.
 - [x] Resource names use the `codebrowser_` prefix.
-- [ ] Evidence records the exact reviewed TASK-005 contract-freeze commit.
+- [x] Evidence records the exact reviewed TASK-005 contract-freeze commit.
 
 ## Evidence
 
@@ -39,3 +39,15 @@ Shared domain, data, Gradle, and manifest contracts remain unchanged. Codex temp
 - Source viewing reuses the reviewed TASK-006A strict classifier with 1 MB input, 200,000-code-point, and 10,000-line display limits. Text is selectable and copyable; truncation, binary, LFS, unsupported UTF-8, oversized, symlink, and submodule states are explicit.
 - Focused code-browser tests and repository-wide `assembleDebug testDebugUnitTest lintDebug` passed: 102 unit tests across 23 suites, zero failures. `git diff --check` and focused sensitive-pattern/read-only scans passed.
 - Device interaction and independent review remain deferred while Codex owns the serial implementation stream.
+
+## Claude independent review closure
+
+Reviewed read-only at commit `16c46a5` per `handoff/claude-master-handoff.md`. Independently reran `assembleDebug testDebugUnitTest lintDebug --rerun-tasks`: BUILD SUCCESSFUL, 102 tests / 23 suites, 0 failures; `git diff --check` clean.
+
+- Root back (verified, no finding): `navigateUp()` returns `false` at the root path instead of calling `loadDirectory`; `MainActivity` treats a `false` result from `onBack` as "leave the code browser" (`codeBrowserViewModel.close(); browsingCode = false`), and both `CodeBrowserScreen` and `RepositoryListScreen` register `BackHandler(onBack = ...)`, so the hardware/gesture back button reaches this logic rather than exiting the Activity directly.
+- Rotation/state restoration (partially verified, tracked not new): the `browsingCode` screen-mode flag survives configuration change via `rememberSaveable`, and `CodeBrowserViewModel`/`RepositoryListViewModel` survive rotation as standard `ViewModel`s. Full typed back-stack/process-death restoration is explicitly out of this task's scope and already tracked as `handoff/claude-master-handoff.md`'s TASK-013; not re-raised here.
+- Immutable snapshots (verified, no finding): `CodeBrowserUiState.Directory.entries` is wrapped in `Collections.unmodifiableList`.
+- P3 (recorded, not fixed): `loadDirectory`/`loadFile` run entirely on `viewModelScope`'s default `Dispatchers.Main.immediate` after the suspending network call resumes — JSON parsing, Base64 decoding, directory sort/dedupe, and `SourceContentClassifier.classify()` (up to 1 MB/200,000 code points) all execute on the main thread. Nothing in this bound is large enough to reliably cause dropped frames, and `AGENTS.md`'s "never block main thread" rule targets network/disk I/O specifically (already off-thread via `OkHttpTransport`'s async callback), so this is not a P0-P2. Recommend wrapping the parse/classify/sort step in `withContext(Dispatchers.Default)` before TASK-016 (release hardening), since that task explicitly targets low-end Android 9 devices and large content. Closure condition: addressed in TASK-016 or a dedicated follow-up, whichever comes first.
+- No P0/P1 found.
+
+Device interaction remains deferred (no physical device available in this environment); this is an existing known limitation, not a new finding.