Complete TASK-022: tolerate incomplete Gitea branch timestamps
Continues Codex's paused in-progress fix in this worktree (confirmed safe to pick up by the human product owner). A missing or malformed branch commit timestamp no longer fails the whole branch page; the latest-branch picker now filters to branches with a known timestamp before selecting the max, and reports Failed (not Empty) when none are known. Adds the one missing test case from the in-progress coverage (all branches with unknown timestamps) plus an explicit test proving the reference screen never depended on the field. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
ca8620958a
commit
b46dd3a72b
@ -110,7 +110,7 @@ class GiteaRepositoryBrowserApi(
|
||||
name = RepositoryRef.of(dto.name.required()),
|
||||
commitId = dto.commit?.id.required(),
|
||||
isProtected = dto.isProtected,
|
||||
commitAtEpochMillis = dto.commit?.timestamp.required().toEpochMillisOrInvalid(),
|
||||
commitAtEpochMillis = dto.commit?.timestamp.toEpochMillisOrNull(),
|
||||
)
|
||||
}
|
||||
return response.toPage(items, page, pageSize)
|
||||
@ -275,6 +275,15 @@ class GiteaRepositoryBrowserApi(
|
||||
throw ApiFailure.InvalidResponse()
|
||||
}
|
||||
|
||||
private fun String?.toEpochMillisOrNull(): Long? =
|
||||
this?.takeIf(String::isNotBlank)?.let {
|
||||
try {
|
||||
Instant.parse(it).toEpochMilli()
|
||||
} catch (_: DateTimeParseException) {
|
||||
null
|
||||
}
|
||||
}
|
||||
|
||||
private fun parseContentsList(body: String): List<ContentsDto> = try {
|
||||
if (!JsonParser.parseString(body).isJsonArray) throw ApiFailure.InvalidResponse()
|
||||
parseList(body)
|
||||
|
||||
@ -15,7 +15,7 @@ data class BranchSummary(
|
||||
val name: RepositoryRef,
|
||||
val commitId: String,
|
||||
val isProtected: Boolean,
|
||||
val commitAtEpochMillis: Long,
|
||||
val commitAtEpochMillis: Long?,
|
||||
)
|
||||
|
||||
data class TagSummary(
|
||||
|
||||
@ -493,12 +493,18 @@ class RepositoryListViewModel(
|
||||
rowBranchJobs[coordinates] = viewModelScope.launch {
|
||||
try {
|
||||
val branches = loadAllPages { page -> browserApi.listBranches(coordinates, page, PAGE_SIZE) }
|
||||
val latest = branches.maxByOrNull { it.commitAtEpochMillis }
|
||||
val latest = branches
|
||||
.filter { it.commitAtEpochMillis != null }
|
||||
.maxByOrNull { requireNotNull(it.commitAtEpochMillis) }
|
||||
updateRow(coordinates) {
|
||||
it.copy(
|
||||
lastPushedBranch = latest?.let { branch ->
|
||||
LastPushedBranchUiState.Loaded(branch.name.value)
|
||||
} ?: LastPushedBranchUiState.Empty,
|
||||
} ?: if (branches.isEmpty()) {
|
||||
LastPushedBranchUiState.Empty
|
||||
} else {
|
||||
LastPushedBranchUiState.Failed
|
||||
},
|
||||
)
|
||||
}
|
||||
} catch (failure: Throwable) {
|
||||
|
||||
@ -157,6 +157,23 @@ class GiteaRepositoryBrowserApiTest {
|
||||
assertEquals("1", branchUrl.queryParameter("page"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `branch timestamps may be missing or malformed without invalidating references`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
response(
|
||||
"""[
|
||||
{"name":"missing-time","commit":{"id":"abc"},"protected":false},
|
||||
{"name":"bad-time","commit":{"id":"def","timestamp":"not-a-date"},"protected":false}
|
||||
]""".trimIndent(),
|
||||
),
|
||||
)
|
||||
|
||||
val branches = api(transport).listBranches(repository, 1, 20)
|
||||
|
||||
assertEquals(listOf("missing-time", "bad-time"), branches.items.map { it.name.value })
|
||||
assertEquals(listOf(null, null), branches.items.map { it.commitAtEpochMillis })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `tree preserves ref and encoded path while returning a local immutable page`() = runTest {
|
||||
val transport = QueueTransport(
|
||||
|
||||
@ -200,6 +200,44 @@ class RepositoryListViewModelTest {
|
||||
assertEquals(LastPushedBranchUiState.Empty, rows.getValue("empty-repo").lastPushedBranch)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `unknown branch timestamps do not hide a valid latest branch`() = runTest(dispatcher.scheduler) {
|
||||
val browser = FakeBrowser(
|
||||
repositories = listOf(page1Of(repo("mixed"))),
|
||||
branchesByRepository = mapOf(
|
||||
"mixed" to listOf(
|
||||
BranchSummary(RepositoryRef.of("unknown"), "a", false, null),
|
||||
BranchSummary(RepositoryRef.of("main"), "b", false, 5_000L),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
val viewModel = viewModel(browser)
|
||||
advanceUntilIdle()
|
||||
|
||||
val row = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
|
||||
assertEquals(LastPushedBranchUiState.Loaded("main"), row.lastPushedBranch)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a repository with branches but no known timestamps reports failed rather than empty`() = runTest(dispatcher.scheduler) {
|
||||
val browser = FakeBrowser(
|
||||
repositories = listOf(page1Of(repo("all-unknown"))),
|
||||
branchesByRepository = mapOf(
|
||||
"all-unknown" to listOf(
|
||||
BranchSummary(RepositoryRef.of("a"), "1", false, null),
|
||||
BranchSummary(RepositoryRef.of("b"), "2", false, null),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
val viewModel = viewModel(browser)
|
||||
advanceUntilIdle()
|
||||
|
||||
val row = (viewModel.state.value as RepositoryListUiState.Content).repositories.single()
|
||||
assertEquals(LastPushedBranchUiState.Failed, row.lastPushedBranch)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a failed per-row branch fetch does not affect other rows and can be retried`() = runTest(dispatcher.scheduler) {
|
||||
val browser = FakeBrowser(
|
||||
|
||||
@ -27,6 +27,17 @@ class RepositoryReferencePresenterTest {
|
||||
assertFalse(branches.last().isSelected)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a branch with an unknown commit timestamp still appears in the reference list`() {
|
||||
val branches = RepositoryReferencePresenter.branches(
|
||||
listOf(BranchSummary(RepositoryRef.of("no-timestamp"), "a", false, null)),
|
||||
"no-timestamp",
|
||||
)
|
||||
|
||||
assertEquals(listOf("no-timestamp"), branches.map { it.name })
|
||||
assertTrue(branches.single().isSelected)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `selecting a tag clears branch selection and selects only that tag`() {
|
||||
val branches = RepositoryReferencePresenter.branches(
|
||||
|
||||
@ -320,3 +320,16 @@ This is the append-only implementation and handoff record shared by Codex and Cl
|
||||
- Root cause: This Gitea server returned `/user/repos` in apparent name order despite the existing `sort=updated&order=desc` query, so trusting server paging could not guarantee that the newest repository appeared on page 1.
|
||||
- Implementation: The ViewModel now loads all server pages in 50-row batches, performs one stable descending sort by `RepositorySummary.updatedAtEpochMillis`, and slices that immutable snapshot into five-row UI pages. Snapshots are scoped by all/favorites/organization selection, reused for next/previous navigation, and invalidated by refresh and logout.
|
||||
- Checks: Focused regression tests and `assembleDebug testDebugUnitTest lintDebug` passed. The rebuilt APK was installed wirelessly; the authenticated first page visibly changed from alphabetical order to recent-update order, all five rows loaded their latest branch labels, and the crash buffer remained empty.
|
||||
|
||||
## 2026-08-26 — TASK-022: tolerate incomplete Gitea branch timestamps
|
||||
|
||||
- Agent and role: Claude, picked up Codex's paused in-progress work in the same worktree (`task-015-notifications`) after the human product owner explicitly confirmed it was safe to continue rather than assuming so.
|
||||
- Commit/diff: continued on `claude/task-015-notifications`; `domain/model/RepositoryModels.kt`, `data/repositories/GiteaRepositoryBrowserApi.kt`, `feature/repositories/RepositoryListViewModel.kt` and three test files.
|
||||
- Features implemented: A repository with a branch whose commit timestamp is missing or malformed no longer fails the whole branch page (row-level latest-branch fetch or the repository reference screen). Root cause (per Codex's incident evidence, repo `CSOT-T8-iPTN-AMA`): `listBranches`'s mapper required every `commit.timestamp` even though a timestamp isn't needed to identify or browse a branch.
|
||||
- How it was implemented: Codex had already landed the core fix (`BranchSummary.commitAtEpochMillis` → nullable, mapper tolerates a missing/malformed value instead of throwing, latest-branch picker filters to non-null timestamps before taking the max) plus two of the needed tests, uncommitted, when this was picked up. Verified `RepositoryReferencePresenter` needed no change (it never reads the timestamp field) and added an explicit test proving that rather than leaving it implicit. Found and filled one real gap in the in-progress test coverage: a repository whose branches are *all* missing timestamps wasn't tested — confirmed the existing code already handles it correctly (reports `LastPushedBranchUiState.Failed`, distinct from `Empty`, rather than silently showing nothing) and added the missing test.
|
||||
- Files changed: `domain/model/RepositoryModels.kt`, `data/repositories/GiteaRepositoryBrowserApi.kt`, `feature/repositories/RepositoryListViewModel.kt`, `data/repositories/GiteaRepositoryBrowserApiTest.kt`, `feature/repositories/RepositoryReferencePresenterTest.kt`, `feature/repositories/RepositoryListViewModelTest.kt`, `tasks/022-tolerate-branch-timestamp-gaps.md`.
|
||||
- Checks and results: `assembleDebug testDebugUnitTest lintDebug --rerun-tasks` BUILD SUCCESSFUL, all unit tests passing, 0 lint errors; `git diff --check` clean; secret-pattern scan of the staged diff clean.
|
||||
- Review findings and resolutions: One gap found and fixed in Codex's in-progress test coverage (the all-unknown-timestamps case) — see above.
|
||||
- Known limitations: Not retested live against `CSOT-T8-iPTN-AMA` (or any repository with a real missing/malformed branch timestamp) on a physical device with this exact fix — Codex reproduced and diagnosed the original crash on-device, but this fix itself has only unit-test coverage so far.
|
||||
- Next recommended task: A physical-device retest of this fix (Codex has working wireless-adb device access per TASK-020/021's evidence). No other roadmap items are open.
|
||||
- Next owner: Codex or the human product owner, for the device retest; otherwise no further items are unassigned.
|
||||
|
||||
33
tasks/022-tolerate-branch-timestamp-gaps.md
Normal file
33
tasks/022-tolerate-branch-timestamp-gaps.md
Normal file
@ -0,0 +1,33 @@
|
||||
# TASK-022: Tolerate incomplete Gitea branch timestamps
|
||||
|
||||
Status: implemented; all required checks pass; physical-device retest pending
|
||||
Owner: Claude (picked up from Codex's paused in-progress work in the same worktree, with the human product owner's explicit confirmation to continue, 2026-08-26)
|
||||
Reviewer: Codex
|
||||
|
||||
## Outcome
|
||||
|
||||
A repository remains browsable when Gitea returns a valid branch name and commit ID but omits or malforms that branch commit's optional timestamp. One incomplete branch must not invalidate the repository's entire branch list.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
- [x] Missing or malformed branch timestamps map to an unknown timestamp without failing the branch page.
|
||||
- [x] The latest-branch indicator selects the newest branch among entries with valid timestamps.
|
||||
- [x] Reference browsing still displays branches whose timestamps are unknown.
|
||||
- [x] Required branch names and commit IDs remain fail-closed.
|
||||
- [x] Build, unit tests, lint, and secret scan pass.
|
||||
- [ ] Physical-device reproduction pass with this exact fix. The original crash was reproduced and diagnosed by Codex on-device (`CSOT-T8-iPTN-AMA`, see Incident evidence); this fix has been verified only by unit test, not by a live retest against that same repository on a device.
|
||||
|
||||
## Incident evidence
|
||||
|
||||
On the authenticated device, `CSOT-T8-iPTN-AMA` failed both its row-level latest-branch request and its repository reference screen. Both flows share `listBranches`, whose mapper previously required every `commit.timestamp` even though timestamp is not necessary to identify or browse a branch.
|
||||
|
||||
## Evidence (completion)
|
||||
|
||||
- `domain/model/RepositoryModels.kt`: `BranchSummary.commitAtEpochMillis` changed from `Long` to `Long?`.
|
||||
- `data/repositories/GiteaRepositoryBrowserApi.kt`: `listBranches`'s mapper now uses a new `String?.toEpochMillisOrNull()` (parses if present and well-formed, else `null`) instead of `.required().toEpochMillisOrInvalid()` for the timestamp only — `name`/`commit.id` still go through `.required()` and remain fail-closed.
|
||||
- `feature/repositories/RepositoryListViewModel.kt`'s `launchRowBranchFetch`: the latest-branch picker now filters to branches with a non-null timestamp before taking `maxByOrNull`, so an unknown-timestamp branch can never hide a real latest branch. When a repository has branches but *none* have a resolvable timestamp, the row now reports `LastPushedBranchUiState.Failed` (distinct from `Empty`, which stays reserved for a genuinely branch-less repository) rather than silently showing nothing.
|
||||
- `feature/repositories/RepositoryReferencePresenter.kt` needed no code change — it never read `commitAtEpochMillis` — but a test was added (`RepositoryReferencePresenterTest`) to make that guarantee explicit rather than implicit.
|
||||
- Tests added: `GiteaRepositoryBrowserApiTest` (missing/malformed timestamp maps to `null`, not a thrown failure), `RepositoryReferencePresenterTest` (a null-timestamp branch still renders and can be selected), `RepositoryListViewModelTest` ×2 (mixed known/unknown timestamps still surface the true latest; all-unknown reports `Failed` not `Empty`) — the all-unknown case was the one gap in Codex's in-progress test coverage when this was picked up.
|
||||
- `./gradlew assembleDebug testDebugUnitTest lintDebug --rerun-tasks`: BUILD SUCCESSFUL, all unit tests passing, 0 lint errors.
|
||||
- `git diff --cached --check`: clean. Secret-pattern scan of the staged diff: no matches.
|
||||
- Not verified: a live retest of repository `CSOT-T8-iPTN-AMA` (or any repository with a real missing/malformed branch timestamp) on a physical device with this exact fix — left open per this session's established practice of disclosing rather than silently skipping device-dependent verification.
|
||||
Loading…
x
Reference in New Issue
Block a user