[piotr-iohk]

Twin: synonymdev/bitkit-ios#1303

Description

The restore form can open when the pubky.app profile is available and its Paykit copy cannot be read. A refresh can recover the stored tags while the open form still holds an empty list. Preserve the restore snapshot’s unread-tags flag through publication so the save keeps those stored tags. 3.1.1 release PR

The follow-up applies the unread state with the profile that was actually accepted and uses that same snapshot for tag edits. If the profile saves and its tag write fails, identity setup completes and displays the existing tag-save warning.

QA Notes

Setup

Run ./gradlew :app:testDevDebugUnitTest --tests '*PubkyRepoTest'. The regression uses a stubbed profile read and publisher with no live backend.

Automated Checks

• Added PubkyRepoTest.kt to verify that a failed initial read followed by a successful refresh and a restore save preserves the stored tags in the published Paykit copy.
• Covers a failed copy read during restore, a stale or failed refresh, and tag editing against the same profile snapshot.
• Covers finishing identity setup and showing the warning after a partial save.

Evidence

Debug Kotlin compilation, Detekt, Android Lint, and all 5,397 local unit tests passed on 147b2fb67. The signed follow-up 7ec67425e passed debug Kotlin compilation, Detekt, Android Lint, and all 5,402 local unit tests.

Models used

• Planning/scoping: GPT-6 (reasoning: Not exposed)
• Implementation: GPT-6 (reasoning: Not exposed), gpt-6.1-sol (reasoning: high)
• Review: gpt-6.1-sol (reasoning: high)

The session exposes the model family. It does not expose the variant or reasoning setting.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The follow-up appears safe to merge; no new blocking issue was found.

Summary

Preserves stored tags when a restore form opened without being able to read them.

• Restoring a profile keeps tags the form could not load.
• Identity setup finishes when only the tag copy fails to save.

No new actionable issues were found. Checks in this review were based on source inspection; tests were not run.

_{Reviews (2) · Last reviewed commit: "fix: keep stored tags after a partial pr..." · Reviewed by Greptile}

[ovitrif]

Triage: milestone 3.2.0. This PR fixes the pubky.app profile restore on master and closes no issue, so it carries the milestone itself. Cherry-picking it into 3.1.1 stays your call.

[talosmachina]

No findings. Carries the restore read's unread-tags flag in RestoreBase and hands it to writeProfile, so a restore save keeps the stored Paykit tags after a later loadProfile reset tagsUnavailable. Reviewed 147b2fb, light tier, static (the Bitkit Android branch is not built on this reviewer's host; CI covers the build).

What I checked, and 3 candidates I ruled out

Read in full: fetchRemoteProfile, publishIdentityProfile, writeProfile, writeProfileOnce, storedCopy, markTagsRead, loadOwnProfile in PubkyRepo.kt
Call sites traced: fetchRemoteProfile and createIdentity (only CreateProfileViewModel), every restoreBase write
CI: detect-changes green; build-and-test, lint and ui-tests still running at review time

Ruled out

• The bug before the fix: copyUnread = tagsUnread || tagsUnavailable with tagsUnread always false on this path; loadOwnProfile sets tagsUnavailable = false on a successful read, so storedCopy returned StoredCopy(null, null) and keptTags became the form's empty list.
• Paykit-only restore (revision = null): that branch runs only after paykitRead.getOrThrow() succeeded, so the default tagsUnread = false is correct there.
• Stale flag across restores: restoreBase is cleared before each read and after each save, so the flag cannot outlive the form that read it.

Merge confidence: 4/5, CI was still running when this was posted; the new PubkyRepoTest case covers the change.

[github-actions]

Regtest APK

Built from 7ec6742 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[jvsena42]

No findings.

• The form-open tagsUnread flag (PubkyRepo.kt:141-146, set at :962 from paykitRead.isFailure) reaches writeProfile (:1075-1079). The save then re-reads the Paykit copy and keeps stored tags plus the form's new ones (keptTags, :1345).
• Tags added in the form survive. Deleted tags can't come back, because the form never showed the stored ones.
• The base is only used for the same publicKey (:1071) and lives in memory, so a wipe and restore of another wallet ignores it.
• A failed copy write leaves the stored tags alone. A failed publish keeps the base for the retry.
• PubkyRepoTest.kt:1152 drives fail-read → loadProfile → createIdentity, which is the real sequence.

Note on changelog.d/next/1884.fixed.md: release-3.1.1 already carries this fix (9eb6a8896) next to #1801. If 3.1.1 ships as cut, the 3.2.0 notes would announce a fix for something 3.1.1 never had. Your call whether to drop it.

Device gate: no device check needed: no UI change, and the state needs a Paykit read to fail at form open and succeed on a background refresh, which needs fault injection. The unit test is the proof. perf: one extra Paykit read at save, only when the form opened unread. CI: 16 pass, 1 pending.

Reviewer model: Claude Fable 5.2 (claude-fable-5-2), Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

[piotr-iohk]

Following up on the approval notes.

• The Android parity note exposed a data-loss path. When a restore save could not reread the Paykit copy, the next edit could publish an empty list over the saved tags. The follow-up keeps the unread state with the accepted profile, so a stale or failed refresh cannot clear it. Tag edits use that same snapshot. Identity setup also shows the existing warning if its tag write fails after the profile saves. Verified commit 7ec67425e
• The repository requires a changelog fragment for each user-facing fix, so the fragment stays with this master PR.

Kotlin compilation, Detekt, Android Lint, and all 5,402 local unit tests passed. GitHub CI is running for the follow-up.

[piotr-iohk]

@greptileai Please review the current head 7ec67425e.

[talosmachina]

Re-review of 7ec6742 after 147b2fb. 0 earlier findings, 0 new. The follow-up installs the load's unread-tags flag together with the profile under one lock, carries the save's own unread state into installSavedProfile so a save whose copy could not be reread leaves the flag set, has tag edits in ProfileViewModel save against one profileEditBase() snapshot, and makes createIdentity report a failed tag write while keeping the identity. Full tier on the diff since 147b2fb, static: the Bitkit Android branch is not built on this reviewer's host, CI covers the build.

What I checked, and 5 candidates I ruled out

Read in full: loadProfile, fetchRemoteProfile, createIdentity, publishIdentityProfile, finishIdentityCreation, saveProfile, writeProfileOnce, unreadTagsAfterCopy, writeProfileCopy, storedCopy, loadOwnProfile, setProfile, installSavedProfile in PubkyRepo.kt; the save path of CreateProfileViewModel; updateTags in ProfileViewModel
Call sites traced: createIdentity (only CreateProfileViewModel), profileEditBase() (EditProfileViewModel, ProfileViewModel), every tagsUnavailable and tagsReadEpoch write
CI: detect-changes and e2e-branch/determine green; build-and-test, lint, ui-tests and build-local still pending at review time

Ruled out

• A load that is stale by the time it lands rewrites the flag: the flag, epoch, revision and profile are set only inside synchronized(profileStateLock) after isCurrentLoad(), and installSavedProfile goes through setProfile, which moves profileWriteGeneration, so a load in flight across a save is skipped.
• The fire-and-forget copy write (tags == baseTags) clears the flag even though it fails: it yields false only when the save reread the copy and no load moved tagsReadEpoch since, the same condition as the removed markTagsRead.
• A load during a save that set unread is overwritten by the save: only when the save wrote the copy itself (wroteTags, epoch passed as null), and then the copy holds exactly the installed tags, so clearing is correct.
• createIdentity now returning a failure for a created identity: its only caller treats PubkyProfileTagsNotSavedError as success plus the existing tag warning, and loadContacts is launched before the return.
• A restore that keeps a stale unread flag from the previous identity: setProfile(null) now resets tagsUnavailable, and fetchRemoteProfile no longer writes the global flag at all; the restore's state lives only in restoreBase.

Merge confidence: 4/5, CI was still running when this was posted; the four new PubkyRepoTest cases and the CreateProfileViewModelTest case cover the change.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: Not exposed (the session does not report it)

[jvsena42]

Re-approving at 7ec67425e. This also closes the gap I'd seen after a save-time read failure. With the copy unreadable at both form open and save, unreadTagsAfterCopy returns true (:1391). installSavedProfile re-arms tagsUnavailable with the profile under the lock (:2637). The next edit's profileEditBase() then rereads and merges instead of writing [].

Checked:

• Locking: the four profileStateLock blocks hold only atomics, volatile writes, isCurrent and the StateFlow set, so nothing suspends under the monitor. The only nesting is the reentrant installSavedProfile → setProfile.
• Stale loads: a load in flight is dropped by the profileWriteGeneration bump. A save's false applies only with an unchanged tagsReadEpoch, or when the save wrote the tags itself.
• Form open and sign-out: fetchRemoteProfile no longer touches the global flag, so opening Restore can't change a signed-in session's flag. setProfile(null) resets it on sign-out and identity change.

Device gate: no device check needed: the new toast path needs fault injection on the Paykit write, and CreateProfileViewModelTest covers it. perf: not relevant.