No task waits on a Codex review; the redesign tasks are confirmed on the product owner's Z Fold7; TASK-016's leftovers are on hold; no biometric reauthentication. Agent: main (opus) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
114 lines
7.2 KiB
Markdown
114 lines
7.2 KiB
Markdown
# TASK-047: Favorite-push notification accuracy (branch + commit)
|
|
|
|
Status: implemented and closed; items 1-2 fixed, item 3 (near-instant delivery) presented to the
|
|
human product owner as a decision and explicitly declined — current 15-minute-plus cadence stays
|
|
Owner: Claude
|
|
Reviewer: Codex
|
|
|
|
## Outcome
|
|
|
|
The human product owner reported, after actually receiving a favorite-push notification live: (1)
|
|
tapping it opened a branch unrelated to what was actually pushed, (2) the notification text just said
|
|
"저장소를 열어 확인해 보세요" with no indication of what changed. Both were real, already-anticipated
|
|
gaps explicitly recorded in TASK-036's own Known limitations (that task tracked only one scalar
|
|
"latest push time" per repository, with no branch or commit attached). Fixed by widening detection to
|
|
carry the branch that actually advanced, and fetching that branch's latest commit for the
|
|
notification text.
|
|
|
|
A third report — notification timing feels irregular and should fire "as soon as a commit lands" —
|
|
is a real platform constraint, not a bug, and is intentionally left unimplemented pending the human
|
|
product owner's choice between real trade-offs (see Known limitations).
|
|
|
|
## Scope
|
|
|
|
- Included: `GetLatestPushEpochMillisUseCase` → `GetLatestPushUseCase`, now returning a new
|
|
`LatestPush(branch, commitId, epochMillis)` instead of a bare `Long?`; `FavoritePushCheckWorker`
|
|
fetches the newly-pushed branch's latest commit (`stat=false`-fast, per TASK-046) and passes it
|
|
through; `FavoritePushNotifier.notifyNewPush` takes the actual pushed `branch` (no longer
|
|
nullable, no longer the repository's *default* branch) and an optional `CommitSummary` for the
|
|
notification body; new/changed notification strings.
|
|
- Excluded: item 3 (delivery latency) — see Known limitations. No change to the 15-minute
|
|
`WorkManager` polling mechanism itself.
|
|
|
|
## Design notes
|
|
|
|
- **`FavoritePushDetector` is unchanged.** It still compares a plain `Map<Coordinates, Long>` —
|
|
the worker now derives that view from `current.mapValues { it.value.epochMillis }` rather than
|
|
the detector needing to know about branches or commits at all. Keeps the one pure,
|
|
no-Android-dependency class exactly as narrow as it was.
|
|
- **The persisted baseline (`LastSeenPushStore`) is unchanged** — still one epoch-millis long per
|
|
repository. Branch and commit are only ever needed at notify-time, for the run that just detected
|
|
a change, so there's nothing to persist beyond what already existed.
|
|
- **The commit fetch is best-effort.** If it fails (network hiccup, or a race where the branch was
|
|
deleted between the branch-list call and this one), the notification still fires using the
|
|
fallback text rather than the whole notification being lost over a secondary lookup.
|
|
- **`stat=false` (TASK-046) is what makes this affordable.** Fetching one extra commit per
|
|
newly-pushed favorited repository, every ~15 minutes, would have been a meaningfully worse idea
|
|
before that fix made single-commit fetches fast; after it, this is a cheap, single-item page
|
|
request.
|
|
|
|
## Acceptance criteria
|
|
|
|
- [x] Tapping the notification opens the repository at the branch that was actually pushed to, not
|
|
the repository's default branch.
|
|
- [x] The notification body names the branch and the latest commit's author and message summary
|
|
(first line only) when the commit fetch succeeds; falls back to the previous generic text if
|
|
it doesn't.
|
|
- [x] Build, unit tests (updated for the new `LatestPush`-returning use case), and lint pass.
|
|
- [ ] Not yet addressed: notification delivery latency/regularity (item 3) — see Known limitations.
|
|
|
|
## Required checks
|
|
|
|
```text
|
|
./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug
|
|
```
|
|
|
|
## Evidence
|
|
|
|
- `domain/usecase/RepositoryBrowseUseCases.kt`: `LatestPush` data class; `GetLatestPushUseCase`
|
|
(renamed from `GetLatestPushEpochMillisUseCase`) returns it via `maxByOrNull`.
|
|
- `core/notifications/FavoritePushCheckWorker.kt`: uses `GetLatestPushUseCase`; for each newly-pushed
|
|
repository, fetches `browser.listCommits(coordinates, push.branch, path = null, page = 1, pageSize = 1)`
|
|
wrapped in `runCatching`, then calls `notifier.notifyNewPush(coordinates, push.branch, latestCommit)`.
|
|
- `core/notifications/FavoritePushNotifier.kt`: `notifyNewPush` signature changed
|
|
(`defaultBranch: RepositoryRef?` → `branch: RepositoryRef`, plus `latestCommit: CommitSummary?`);
|
|
intent extras are now always attached (a repository that produced a `LatestPush` always has a real
|
|
branch, unlike the old default-branch-may-be-null case); `BigTextStyle` added so a longer commit
|
|
message summary isn't clipped.
|
|
- `strings_favorite_push.xml`: title now includes the branch; content shows `author: message`; a
|
|
separate `_fallback` string covers the best-effort-failed case.
|
|
- `RepositoryBrowseUseCasesTest.kt`: three tests updated to assert on `LatestPush` values instead of
|
|
a bare `Long?`.
|
|
- `./gradlew compileDebugKotlin compileDebugUnitTestKotlin testDebugUnitTest assembleDebug lintDebug`:
|
|
BUILD SUCCESSFUL, all unit tests passing, 0 lint errors.
|
|
|
|
## Known limitations
|
|
|
|
- **Item 3 (near-instant delivery) is not implemented.** `WorkManager`'s `PeriodicWorkRequest` has a
|
|
hard, non-configurable 15-minute floor (`PeriodicWorkRequest.MIN_PERIODIC_INTERVAL_MILLIS`) —
|
|
Android itself enforces this, not a choice this app makes. Actual elapsed time between runs can
|
|
exceed 15 minutes further still, unevenly, whenever Doze/App Standby defers a low-priority
|
|
periodic job — which is almost certainly the "불규칙함" the human product owner is seeing, not a
|
|
bug in this app's own scheduling code. Closing this gap for real means picking one of:
|
|
1. A foreground service polling much more frequently (e.g. every ~1 minute) — bypasses Doze
|
|
throttling, but requires a permanently visible notification (Android mandates one for any
|
|
foreground service) and continuously drains battery for as long as the app is "running" this
|
|
way.
|
|
2. A real push architecture: a Gitea webhook (configured server-side, which the human product
|
|
owner can do since they administer this self-hosted instance) firing on `push`, received by
|
|
some always-on endpoint, relayed to the device via Firebase Cloud Messaging (or an equivalent
|
|
push service) — genuinely near-instant, low battery cost, but a real one-time infrastructure
|
|
build: a webhook receiver running somewhere reachable (plausibly on the same Synology NAS), a
|
|
Firebase project, and `google-services.json`/FCM SDK integration into this app.
|
|
3. Leave the 15-minute-plus, Doze-affected cadence as-is.
|
|
This decision needed the human product owner, not just Claude: option 2 requires action outside
|
|
this codebase (their Gitea admin panel, a Firebase account) that only they can do, and option 1's
|
|
persistent-notification/battery trade-off is a real product call, not an engineering default to
|
|
assume silently. Presented directly as a three-way choice; the product owner chose to keep the
|
|
current cadence as-is ("지금이 대로 괜찮음") rather than pay the battery/persistent-notification
|
|
cost of a foreground service or take on the webhook+FCM infrastructure work.
|
|
|
|
## Review findings
|
|
|
|
None yet — no Codex review pending (Codex retired, 2026-09-29).
|