Skip to content

feat: use the pubky.app profile directly - #1235

Merged
ovitrif merged 27 commits into
masterfrom
feat/pubky-app-profile
Nov 11, 2026
Merged

ovitrif merged 27 commits into
masterfrom
feat/pubky-app-profile

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1287, closes #1243

Twin: synonymdev/bitkit-android#1801
Dependency: pubky/paykit-rs#270

This PR makes Bitkit read and edit the user's own pubky.app profile (/pub/pubky.app/profile.json) directly, so Bitkit and pubky.app share one profile instead of Bitkit keeping its own.

It needs pubky/paykit-rs#270, released as Paykit 0.1.1-rc7, and pins that version.

Description

  • Loads the signed-in profile from pubky.app with no fallback to Bitkit's own profile, so an edit made in pubky.app shows in Bitkit.
  • Moves a Bitkit-only profile to pubky.app on first load: line breaks become spaces, text is trimmed to pubky.app's limits (name 50, bio 160, five links), links without a scheme become full https:, mailto: or tel: URLs when they look like a site, an email address or a phone number, and the avatar is re-uploaded as a pubky.app file (retried on later loads while pubky.app has no image); one already on pubky.app is kept as is. While the move cannot finish, for example for a name under 3 characters, the Bitkit profile stays visible and the next valid save creates the pubky.app profile, fitted to pubky.app's limits the same way, so a restored profile with a long bio or extra links still saves. A profile that already exists on pubky.app is used as is. One deleted on pubky.app after Bitkit saw it is not recreated: the identity goes to profile setup.
  • Saves only the fields the user changed, at the revision the edit started from; the first save after creating or restoring a profile included. When the profile changed in pubky.app meanwhile, the save is refused, Bitkit reloads the profile and asks the user to make the edit again, so a pubky.app edit is never overwritten.
  • Keeps every pubky.app field Bitkit does not edit, such as status and fields other apps add.
  • Uploads new profile avatars as pubky.app blobs and files, so pubky.app shows them. Contact avatars stay on Bitkit's Paykit storage.
  • Asks for write access to the pubky.app profile, blobs and files in the same session Bitkit already creates, so there is no second approval. Sessions from earlier builds keep restoring; the first pubky.app write signs in again once to gain the access.
  • Delete Profile now also deletes the pubky.app profile of a wallet-owned identity, as its dialog says; unlinking a Ring identity leaves that profile in place. A load or save still under way when the profile is deleted writes neither profile back.
  • Keeps Bitkit's own profile tags in its Paykit profile copy; a failed read of that copy no longer erases them, a load running alongside a save does not write the old tags back, an edit that began while the copy could not be read keeps the stored tags, and a tag change that is not saved is reported while the other changes are kept. A save with no tag change returns after the pubky.app write and lets the copy follow in the background, so it takes one write.
  • Limits names to pubky.app's 3 to 50 characters and new bio text to 160, counted as pubky.app counts them, and stops Add Link at 5 links.
  • Profile reloads each time it opens, so a pubky.app edit shows without a relaunch.
  • Replaces the Edit Profile note: it now says this is the user's public pubky.app profile and that changes show on pubky.app.

Out of Scope

  • Profile tags: pubky.app's profile has no tags field, so Bitkit keeps its own profile tags in its public Paykit profile (/pub/paykit/profile.json), which Bitkit keeps writing in line with pubky.app for other Paykit apps.
  • Contacts: contact profiles, tags and edits stay Bitkit-local as they are.
  • A profile deleted on pubky.app comes back after a reinstall or wallet restore, and an avatar still to move loses its reference, as both records are per install: tracked in bug: deleted pubky.app profile returns after reinstall #1240 (thread).
  • Link values that are free text, such as a city, are not links pubky.app can hold, so a move or a restore leaves them out: tracked in bug: free-text profile links dropped when moving to pubky.app #1241.
  • Profile copy: the new Edit Profile note and the changed-in-pubky.app message wait for design review.

Design

N/A — no design available.

Preview

Recordings are in the device-test comment: #1235 (comment)

QA Notes

Setup

  • Staging homeserver and staging.pubky.app; a disposable wallet.

Journeys

  • J1: new edit-pubky-app-profile.xml — the Edit Profile note names the public pubky.app profile, and a name edit saves and survives a relaunch

Manual Tests

  • 1. Edit Profile → change the name → Save → open the same profile on staging.pubky.app → it shows the new name, and its status and other fields are unchanged — pubky.app in a browser not in Capabilities
  • 2. Open Edit Profile → change the bio on staging.pubky.app → change the name in Bitkit → Save → Bitkit shows the changed-in-pubky.app error and reloads the new bio; pubky.app's bio is not overwritten — pubky.app in a browser not in Capabilities
  • 3. Wallet with a Bitkit profile from master and no pubky.app profile → update to this build → open Profile → staging.pubky.app shows the same name, bio, links and avatar — pubky.app in a browser not in Capabilities
  • 4. Identity that already has a pubky.app profile and a different Bitkit profile → update to this build → Bitkit shows the pubky.app profile — pubky.app in a browser not in Capabilities
  • 5. Edit Profile → pick a new avatar → Save → staging.pubky.app shows the new avatar — pubky.app in a browser not in Capabilities
  • 6. Edit Profile → Delete Profile → confirm → staging.pubky.app no longer has the profile, and the Bitkit copy at /pub/paykit/profile.json is gone — pubky.app in a browser not in Capabilities
  • 7. With Profile open earlier in the session, change the bio on staging.pubky.app → open Profile again → Bitkit shows the new bio without a relaunch — pubky.app in a browser not in Capabilities

Automated Checks

  • added PubkyAppProfileStoreTests.swift — a save writes only changed fields, clears a removed image or links, and writes every field when nothing was loaded; link rules (sites, email, phone, free text), Cc-only flattening and scalar caps; ProfileWriteGateTests — a write begun before a deletion is refused, and a copy refresh read before or during a save is refused
  • updated Paykit test fixtures for the 0.1.1-rc3 to 0.1.1-rc6 record fields

Evidence

Gaps

  • Manual test 5 (avatar) ran with a debug-only local hook in place of the photo picker, since the test simulator has no photo library service; the picker itself is not exercised on iOS.
  • The revision, pending-avatar and tag paths go through static PubkyService calls the iOS unit tests cannot fake; feat: use the pubky.app profile directly bitkit-android#1801 has unit tests for the same logic.

Models used

  • Planning/scoping: claude-opus-5-5 (reasoning: Not exposed)
  • Implementation: claude-opus-5-5 (reasoning: Not exposed)
  • Review: Not performed

Metadata note: the session reports its model but not a reasoning setting.

@ovitrif ovitrif self-assigned this Nov 6, 2026
@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 5003641: contact avatars stay on Bitkit's Paykit storage, a profile deleted on pubky.app after Bitkit saw it is no longer recreated, migration publishes text before the avatar and drops links pubky.app would reject, and a load that a save overtook no longer keeps an older revision.

@ovitrif

ovitrif commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

QA for 08f6ce8:

Tested on an iPhone 17 simulator (iOS 27), Debug build against staging, with a local Paykit build of pubky/paykit-rs#270. Where a test needs pubky.app, the stored profile was read from the homeserver, and the "edit in pubky.app" was a write of profile.json with the wallet's own key.

🟢 Create and edit a profile, Edit the pubky.app profile
Create and edit a profile

On a new wallet, a profile named iOS Fresh Test is saved to pubky.app, and renaming it to iOS Edited updates pubky.app.

case1-fresh2x.mp4

Edit the pubky.app profile

Edit Profile shows the new pubky.app note, and the edited name shows on Profile; the same recording.


🟢 Upgrade a wallet whose profile is only in Bitkit
Upgrade a wallet whose profile is only in Bitkit

A master build saves iOS Master Profile with a bio only to Bitkit's Paykit copy, and pubky.app has no profile. After updating to this build, opening Profile moves the name and bio to pubky.app. A rename then saves, which is the first write with the old session.

case2a-upgrade-no-pubkyapp2x.mp4

🟢 Upgrade a wallet that already has a pubky.app profile
Upgrade a wallet that already has a pubky.app profile

A master build saves iOS Bitkit Only, then pubky.app gets a profile named iOS From Pubky App with status away. After updating, Bitkit shows the pubky.app name. Adding a bio saves it and keeps the name and status.

case2b-upgrade-pubkyapp-exists2x.mp4

🟢 A profile changed in pubky.app during an edit
A profile changed in pubky.app during an edit

While Edit Profile is open, pubky.app gets a new bio. Saving a new name shows "Your profile changed in pubky.app", reloads the form with the new bio and leaves pubky.app unchanged. Saving again stores the name and keeps the bio and status.

case3-conflict2x.mp4

Device tests skipped ⚠️:

@ovitrif
ovitrif marked this pull request as ready for review November 6, 2026 18:48
@ovitrif
ovitrif requested review from a team, coreyphillips and pwltr and removed request for a team November 6, 2026 18:48
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T18:51:13.814617Z 08f6ce8 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08f6ce8f3e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps

greptile-apps Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 2/5

[Medium impact] The change affects public profile reads and writes for signed-in Bitkit users.

Fix the avatar migration and profile overwrite paths before merging.

Findings

  1. P1 Migration can lose the photo ▶
  2. P1 Restore overwrites newer profile details ▶
  3. P1 Refresh lets stale fields overwrite ▶
  4. P2 Forms allow unsupported profile values ▶

Summary

Bitkit now reads and edits the shared pubky.app profile.

  • Bitkit reads and edits the same profile that appears on pubky.app.
  • New sessions get pubky.app access while contact pictures stay on Bitkit storage.

The review found avatar loss during a failed migration and two paths that can overwrite newer profile fields. Normal saves also lack the stated field limits.

ovitrif explicitly acknowledges that Paykit publication and the Package.resolved update are pending. The author also leaves Delete Profile behavior and final copy review out of scope.

Checks were static. No iOS build, runtime test, or visual capture was performed in this Linux environment.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Load[Load own profile] --> Read[Read pubky.app profile and revision]
    Read --> Exists{Profile exists?}
    Exists -->|Yes| Base[Cache profile and revision]
    Exists -->|No, eligible legacy profile| Migrate[Move Paykit profile and avatar]
    Migrate --> Base
    Base --> Form[Show edit form]
    Form --> Save[Compare fields with cached base]
    Save --> Write[Write with expected revision]
    Write -->|Conflict| Reload[Reload and ask user to edit again]
    Write -->|Success| Mirror[Update Paykit copy with tags]
Loading

Reviews (1) · Last reviewed commit: "fix: save a multi-line bio as one line" · Reviewed by Greptile

@talosmachina talosmachina left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 findings (1 P1, 1 P2). The iOS twin of synonymdev/bitkit-android#1801: the own profile is read and written on pubky.app through PubkyAppProfileStore, with revision checks and a one-time move of a Bitkit-only profile. Reviewed 08f6ce8, full tier, static: iOS does not build on Linux.

P2: names under 3 characters are still accepted by the forms. The description says names are limited to pubky.app's 3 to 50 characters, but ProfileEditFormView.swift:297 and CreateProfileView.swift:67 only disable Save on an empty name, and nothing in the diff checks the minimum. A 1 or 2 character name reaches the SDK, which refuses it (NAME_MIN_CHARS = 3 in pubky/paykit-rs#270), and the user sees a generic save error. The Android twin gates Save with isValidProfileName.

What I checked, and 3 candidates I ruled out

Read in full: PubkyAppProfileStore.swift, the PubkyProfileManager.swift and PubkyService.swift changes, EditProfileView.swift, CreateProfileView.swift remote lookup, and the SDK side in pubky/paykit-rs#270.

Ruled out

  • Stale base after a concurrent save: saveCount keeps a load that raced a save from overwriting the newer base.
  • Re-migrating a profile deleted on pubky.app: pubkyAppProfileOwners stops it once one was seen.
  • Edited multi-line bio: save replaces \n since 08f6ce8.

Merge confidence: 3/5, one P1.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two independent reviews.

needs changing before merge

  • A retried tag edit overwrites the pubky.app edit it was refused for (Bitkit/Managers/PubkyAppProfileStore.swift:90)
  • Valid Bitkit profiles fail to move and leave Profile unusable (Bitkit/Managers/PubkyAppProfileStore.swift:128-129)
  • The pin to unpublished Paykit rc7 fails package resolution in CI (Bitkit.xcodeproj/project.pbxproj:1212)
  • A failed avatar migration permanently removes the saved avatar (Bitkit/Managers/PubkyAppProfileStore.swift:158-160)
  • Legacy profiles with short names cannot load after upgrade (Bitkit/Managers/PubkyAppProfileStore.swift:127-129)
  • A failed profile-copy read causes existing tags to be erased (Bitkit/Managers/PubkyAppProfileStore.swift:52-54)
  • Migration deletes supported website, email and phone links (Bitkit/Managers/PubkyAppProfileStore.swift:122-126)

worth doing, does not block

  • A failed avatar move loses the avatar reference for good (Bitkit/Managers/PubkyAppProfileStore.swift:160)
  • A failed Paykit read makes the next save erase the profile tags (Bitkit/Managers/PubkyAppProfileStore.swift:74)
  • Tag edits report success when their only store rejects them (Bitkit/Managers/PubkyAppProfileStore.swift:110)
  • A multi-line bio stored on pubky.app blocks every Bitkit save (Bitkit/Managers/PubkyAppProfileStore.swift:194)
  • The name limit is not implemented and pubky.app limits only fail at Save (Bitkit/Components/ProfileNameField.swift:18). The PR says it limits names to 3 to 50 characters, but iOS has no such limit: ProfileNameField is unbounded and both profile views only reject an empty name. An out-of-range name, a bio over 160 characters, a sixth link, or a bio with a tab or pasted CRLF all reach the SDK and come back as an SDK error string in the toast. The \n replacement leaves \r behind for CRLF, which I checked by running it. On a new wallet, a two-letter name fails after signup, and completeIdentityCreation then discards the session. Bound these fields in the form so the limit is visible before Save.
  • A pubky.app edit does not show in Bitkit until the next cold start (Bitkit/Views/Profile/ProfileView.swift:294). The PR says a pubky.app edit shows in Bitkit, but while the app stays running it only appears after a refused save. refreshProfile returns as soon as profile is set, and the other loadProfile callers are session restoration and the Edit Profile conflict path. Long-lived sessions therefore hold stale data, which also makes the retry overwrite above easier to hit. Refresh when Profile appears or on foreground.
  • A profile deleted on pubky.app dead-ends at the load error (Bitkit/Managers/PubkyProfileManager.swift:1199). After a relaunch, a wallet-derived identity whose pubky.app profile was deleted can only sign out. load throws profileNotFound and loadProfile only logs it, so profile stays nil and Edit is unreachable, even though save could recreate the profile. Only the Ring refresh path treats not-found as profile setup. Traced through the code, not run. Route this case to profile setup or Edit Profile.
  • The Delete Profile dialog still promises removal from Pubky (Bitkit/Resources/Localization/en.lproj/Localizable.strings:1195)

the reviewers disagree, your call

  • Signed-out Create Profile no longer restores a Bitkit-only profile (Bitkit/Views/Profile/CreateProfileView.swift:186). Objection: Real, but blocking rather than non-blocking: Signed-out lookup cannot return a Bitkit-only profile, so creation proceeds with empty fields and replaces its stored contents. Without a loaded base, creation also explicitly clears an existing pubky.app bio and links.
  • Conflict recovery can let a retry overwrite external edits (Bitkit/Managers/PubkyAppProfileStore.swift:103-105)

Comment thread Bitkit.xcodeproj/project.pbxproj Resolved
@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 5d805c4 for the Codex and Greptile findings: Delete Profile now also deletes the pubky.app profile of a wallet-owned identity; a failed avatar move keeps the original avatar and retries; the restore prefill reads pubky.app; a refresh keeps the revision an open edit started from; the profile screens enforce pubky.app's name and bio limits. Needs pubky/paykit-rs#270 for the new delete call.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 1eaabe4 for @coreyphillips's review and Talos's comment: saves carry the revision their data was built from; migration flattens line breaks, truncates by Unicode scalars and keeps scheme-less links as full URLs; while a move cannot finish the Bitkit profile stays usable and the next valid save creates the pubky.app profile; a failed Paykit read no longer erases tags, and a failed tag write is reported. pubky/paykit-rs#270 now validates only the fields an update writes. The rc7 pin stays a merge gate until rc7 ships.

@ovitrif
ovitrif dismissed coreyphillips’s stale review November 6, 2026 19:56

Addressed in 5d805c4 and 1eaabe4, plus pubky/paykit-rs#270 d01659da; the rc7 pin thread stays open as the merge gate.

@ovitrif
ovitrif requested a review from coreyphillips November 6, 2026 19:56

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked after 2 new commits (5d805c4, 1eaabe4). 6 points still block a merge.

addressed

  • A retried tag edit overwrites the pubky.app edit it was refused for (Bitkit/Managers/PubkyAppProfileStore.swift:90). Fixed: ProfileView.swift:283 now sends the shown profile's revision, and PubkyAppProfileStore.swift:105-111 diffs against that revision and sends it. I ran the store source against a fake porting Paykit's revision check: the tag save and the retry from the old screen both threw changedElsewhere, and the pubky.app bio survived.
  • Valid Bitkit profiles fail to move and leave Profile unusable (Bitkit/Managers/PubkyAppProfileStore.swift:128-129). Fixed: migrate now flattens text and truncates by scalars (PubkyAppProfileStore.swift:179-184), and load returns the Bitkit profile while the move cannot finish (lines 76-79). In the same harness LF, CRLF, tab and 100 skin-tone emoji bios all moved, and Al loaded with no error.
  • A multi-line bio stored on pubky.app blocks every Bitkit save (Bitkit/Managers/PubkyAppProfileStore.swift:194). Fixed: validate_pubky_profile_update in paykit-rs d01659da checks only the fields an update names. I ran cargo test -p paykit-sdk --lib pubky_profile at that commit and all 10 passed, including test_pubky_profile_merge_ignores_stored_fields_it_does_not_write; it reaches Bitkit through rc7, tracked by point 3.
  • The Delete Profile dialog still promises removal from Pubky (Bitkit/Resources/Localization/en.lproj/Localizable.strings:1195). Fixed: PubkyProfileManager.swift:170-175 now deletes the pubky.app profile for a wallet-owned identity via PubkyAppProfileStore.swift:228-236, and a Ring identity gets the unlink copy from the same isRingAdopted check (EditProfileView.swift:23). I traced this into delete_pubky_profile and its Swift binding at d01659da, but did not run it against a homeserver.
  • Conflict recovery can let a retry overwrite external edits (Bitkit/Managers/PubkyAppProfileStore.swift:103-105). Fixed: The retry carries the form's revision (EditProfileView.swift:158,244), and a failed editor reload leaves that revision in place. With the store reloaded and the form unchanged, the harness refused the retry and kept the pubky.app bio.
  • Legacy profiles with short names cannot load after upgrade (Bitkit/Managers/PubkyAppProfileStore.swift:127-129). Fixed: load returns the Bitkit profile for a short name (PubkyAppProfileStore.swift:76-79), so canEditProfile holds and Edit Profile opens. In the harness Al loaded and a save with a valid name created the pubky.app profile; what that save does to the avatar is a new finding.
  • Migration deletes supported website, email and phone links (Bitkit/Managers/PubkyAppProfileStore.swift:122-126). Fixed: linkUrl (PubkyAppProfileStore.swift:311-325) runs in migrate and on every save. In the harness example.com, alice@example.com and +49123456789 moved as https:, mailto: and tel: URLs; other free-text values are a new finding.

still open

  • The pin to unpublished Paykit rc7 fails package resolution in CI (Bitkit.xcodeproj/project.pbxproj:1212). The project pins Paykit 0.1.1-rc7, which does not exist yet, and Package.resolved still says rc2, so the app does not build from a clean checkout. git ls-remote shows v0.1.1-rc6 as the newest tag, and pubky/paykit-rs#270 is still open. The unit-tests run for 08f6ce8 fails at Resolve Swift packages with an out-of-date resolved file was detected. The PR discloses this, but it is marked ready, so it is called out here as a merge gate. Hold the merge until rc7 is published and Package.resolved is updated. Rerun the unit suite then, as the last recorded run was on 5003641, before the final commit. git ls-remote still stops at v0.1.1-rc6, pubky/paykit-rs#270 is open, and Package.resolved:37 says rc2 against the rc7 pin at project.pbxproj:1212. You agree this is a merge gate. I could not build the app either, so my checks ran the store and model sources against fakes.
  • A failed avatar move loses the avatar reference for good (Bitkit/Managers/PubkyAppProfileStore.swift:160). If the avatar step of migrate fails once, the avatar URI is erased and the move is never retried. migrate returns the text-only snapshot (line 160). load then sees it differs from the Paykit copy, and publishCopy writes image: nil over the only reference (line 79). Later loads find a pubky.app profile, so they never migrate. Reproduced with the store source and a fake blob read that failed once: both loads returned no image and the Paykit copy's image was nil. Keep the copy's image until the avatar has moved, or retry the avatar on a later load. The load path now retries, but a save during the pending window still publishes the snapshot's nil image over the Paykit copy (PubkyAppProfileStore.swift:128-134). In the harness a name edit after one failed move left the copy and pubky.app with no image, so the next load had nothing to retry and the pending flag stayed set.
  • A failed Paykit read makes the next save erase the profile tags (Bitkit/Managers/PubkyAppProfileStore.swift:74). load tolerates a failed Paykit read (line 53) and then builds the profile with tags: [] (line 74), so the next save overwrites the stored tags with nothing. Reproduced with the store source: after one failed read the load returned no tags while the copy held friend and work. A name edit then left the copy with none. Tags have no other store, so a failed read should not look like an empty list. This holds for one save only. Line 158 clears tagsUnavailable while the manager commits the form's empty tags (PubkyProfileManager.swift:1028), so the second save wrote an empty list over friend and work in the harness, and if the re-read in tagsToKeep also fails the first save erases them.
  • Tag edits report success when their only store rejects them (Bitkit/Managers/PubkyAppProfileStore.swift:110). publishCopy swallows its error (line 171), which suits a mirror, but the Paykit copy is the only store for tags. When that publish fails, save returns normally, the manager commits the tag and the user sees it saved. Reproduced with the store source: the save reported success and the next load returned no tags. On master this publish error reached the user. Surface the failure when the tags changed. Codex still sees it: The first failure is surfaced, but remember updates the base before the copy write (Bitkit/Managers/PubkyAppProfileStore.swift:129-137). I reproduced a tag-only retry at the same pubky.app revision: its failed write returned success because the remembered base already contained the unsaved tags.
  • Signed-out Create Profile no longer restores a Bitkit-only profile (Bitkit/Views/Profile/CreateProfileView.swift:186). With no session or stored key, CreateProfileView.remoteProfile calls fetchRemoteProfile, which now goes through store.load. That reads the Paykit profile but cannot return it, and migrate needs a signed-in session. Sign-out clears both keychain entries, so a user who signed out on master gets a blank form instead of the restore flow. Continue then publishes an empty bio, links and tags, and publishCopy replaces the Bitkit profile with them. The same base == nil branch writes bio: "" and links: []. If the signed-out lookup fails transiently for an identity that has a pubky.app profile, Continue clears that bio and those links at a fresh revision. Traced through the code, not run. CreateProfileView.swift:190-191 is unchanged for a signed-out user, and load can only return a Bitkit profile when signed in (line 70). In the harness the signed-out load threw profileNotFound and Continue replaced the copy with an empty bio, links and tags; that branch also still swallows a transient lookup failure.
  • The name limit is not implemented and pubky.app limits only fail at Save (Bitkit/Components/ProfileNameField.swift:18). The PR says it limits names to 3 to 50 characters, but iOS has no such limit: ProfileNameField is unbounded and both profile views only reject an empty name. An out-of-range name, a bio over 160 characters, a sixth link, or a bio with a tab or pasted CRLF all reach the SDK and come back as an SDK error string in the toast. The \n replacement leaves \r behind for CRLF, which I checked by running it. On a new wallet, a two-letter name fails after signup, and completeIdentityCreation then discards the session. Bound these fields in the form so the limit is visible before Save. Names are gated, name and bio are capped, and tabs and CRLF are flattened, but a sixth link is still only refused at Save (ProfileEditFormView.swift:56 appends with no limit). The caps also count graphemes while Paykit counts scalars, so 100 skin-tone emoji pass the form and fail in the SDK, and a short name disables Save with no explanation.
  • A pubky.app edit does not show in Bitkit until the next cold start (Bitkit/Views/Profile/ProfileView.swift:294). The PR says a pubky.app edit shows in Bitkit, but while the app stays running it only appears after a refused save. refreshProfile returns as soon as profile is set, and the other loadProfile callers are session restoration and the Edit Profile conflict path. Long-lived sessions therefore hold stale data, which also makes the retry overwrite above easier to hit. Refresh when Profile appears or on foreground. ProfileView.swift:298 still returns once profile is set, and foreground recovery still skips a signed-in session (PubkyProfileManager.swift:269). The only new reload follows a refused tag edit.
  • A profile deleted on pubky.app dead-ends at the load error (Bitkit/Managers/PubkyProfileManager.swift:1199). After a relaunch, a wallet-derived identity whose pubky.app profile was deleted can only sign out. load throws profileNotFound and loadProfile only logs it, so profile stays nil and Edit is unreachable, even though save could recreate the profile. Only the Ring refresh path treats not-found as profile setup. Traced through the code, not run. Route this case to profile setup or Edit Profile. load still throws profileNotFound at PubkyAppProfileStore.swift:80 and loadProfile only logs it, so Edit stays unreachable after a relaunch. In a running session it now also loops: the harness refused every save as changed in pubky.app, because a carried revision can never match a missing profile, and the reload failed.
  • A failed avatar migration permanently removes the saved avatar (Bitkit/Managers/PubkyAppProfileStore.swift:158-160). An avatar upload failure permanently discards the legacy avatar reference. This catch returns the image-less profile as a completed migration. load then records the owner and overwrites the Paykit copy without its image. I reproduced one failed upload followed by a healthy reload. Both profiles stayed without an image, and the upload was never retried. One failed upload followed by a load now retries and keeps the image, which the harness confirmed. A save before that retry still writes nil over the Paykit copy (PubkyAppProfileStore.swift:128-134), and then the upload never runs, so the saved avatar is still lost for good.
  • A failed profile-copy read causes existing tags to be erased (Bitkit/Managers/PubkyAppProfileStore.swift:52-54). A temporary Paykit-profile read failure can erase existing tags on the next edit. This catch turns the failure into nil, so the successful pubky.app load returns tags: []. That becomes the editor's tag list and is later written over the Paykit profile. I reproduced one failed read for a profile tagged vip, followed by a name-only save. The persisted tags became empty. The stored tags survive only the first save after a failed read. In the harness vip was erased at once when the re-read in tagsToKeep also failed, and friend and work were erased by the second save because the manager keeps the form's empty list (PubkyProfileManager.swift:1028).

new since the last review

needs changing before merge

  • First save after create or restore overwrites pubky.app edits (Bitkit/Managers/PubkyProfileManager.swift:765-776). The first save after Bitkit creates or restores a profile carries no revision, so it writes every field over whatever pubky.app holds, with no conflict check. createProfile commits a PubkyProfile with no revision (PubkyProfileManager.swift:765-776), and writeProfile at line 762 sends none. Nothing reloads it, because refreshProfile returns once profile is set. With baseRevision nil, save fetches the latest revision and calls update(from:base:nil) (PubkyAppProfileStore.swift:112-116), so name, bio and links are written at a fresh revision and can never be refused. 1eaabe4 caused this by removing bases, so the reply on the restore thread that the save diffs against what was shown no longer holds. The fallback at PubkyProfileManager.swift:905 has a related gap: a form opened with no revision can take a newer one than its data. Steps: create a profile in Bitkit, add a bio and a link on pubky.app, return without relaunching, add a tag. Confirmed by running the store source against a fake porting Paykit's revision check at d01659da. The tag add succeeded and removed the pubky.app bio and link. In the restore flow, Continue put the old bio back over the pubky.app edit. The manager steps are traced from the lines above, not run. Set the revision on the created profile, pass the restored profile's revision into the save, and refuse a save with no revision when a pubky.app profile exists.
  • Any save rewrites joined emoji and Persian text in untouched fields (Bitkit/Managers/PubkyAppProfileStore.swift:300-303)
  • New profiles overwrite external changes on their first edit (Bitkit/Managers/PubkyAppProfileStore.swift:112-116)

worth doing, does not block

  • Free-text links are dropped or rewritten as mail and phone links (Bitkit/Managers/PubkyAppProfileStore.swift:311-325)
  • A move finished by a save leaves the avatar on Paykit storage (Bitkit/Managers/PubkyAppProfileStore.swift:261-265)
  • A retried avatar move overwrites an avatar set on pubky.app (Bitkit/Managers/PubkyAppProfileStore.swift:73-75)
  • A failed Paykit read makes Create Profile treat a profile as missing (Bitkit/Managers/PubkyAppProfileStore.swift:60-64)
  • Profile save commits after an await with no session re-check (Bitkit/Managers/PubkyProfileManager.swift:1017-1032)
  • Edit Profile silently cuts a stored bio longer than 160 characters (Bitkit/Views/Profile/EditProfileView.swift:70-74)
  • A pending avatar move overwrites a newer pubky.app avatar (Bitkit/Managers/PubkyAppProfileStore.swift:73-74)

nits

  • PR description and evidence predate the last two commits [one reviewer only]. The Out of Scope bullet still says Delete Profile leaves the pubky.app profile in place, which 5d805c4 changed. Evidence stops at 5003641 for unit tests and 08f6ce8 for device runs, and no QA case covers delete. These two commits add one test, for linkUrl, so the revision, pending avatar and tag paths have no coverage.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

@coreyphillips thanks for the recheck. Fixes for all six blocking points and the other findings are in progress and land with the next push, with Android parity on synonymdev/bitkit-android#1801. I'll reply in each thread once it is up.

@coreyphillips coreyphillips left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked after replies, with no new commit since 1eaabe4. 6 points still block a merge.

addressed

  • Tag edits report success when their only store rejects them (Bitkit/Managers/PubkyAppProfileStore.swift:110). Fixed: PubkyAppProfileStore.swift:131-137 rethrows the copy failure when tags changed, and publishProfileEdit and ProfileView.updateTags carry it to the toast. I ran the store with a failing copy write: the save threw, the copy kept the old tags and the retry stored the new tag.

still open

  • The pin to unpublished Paykit rc7 fails package resolution in CI (Bitkit.xcodeproj/project.pbxproj:1212). The project pins Paykit 0.1.1-rc7, which does not exist yet, and Package.resolved still says rc2, so the app does not build from a clean checkout. git ls-remote shows v0.1.1-rc6 as the newest tag, and pubky/paykit-rs#270 is still open. The unit-tests run for 08f6ce8 fails at Resolve Swift packages with an out-of-date resolved file was detected. The PR discloses this, but it is marked ready, so it is called out here as a merge gate. Hold the merge until rc7 is published and Package.resolved is updated. Rerun the unit suite then, as the last recorded run was on 5003641, before the final commit. Paykit v0.1.1-rc7 is still not tagged (rc6 is newest by git ls-remote), pubky/paykit-rs#270 is open and Package.resolved pins rc2. Run Tests on 1eaabe4 fails at package resolution with an out-of-date resolved file was detected.
  • A failed avatar move loses the avatar reference for good (Bitkit/Managers/PubkyAppProfileStore.swift:160). If the avatar step of migrate fails once, the avatar URI is erased and the move is never retried. migrate returns the text-only snapshot (line 160). load then sees it differs from the Paykit copy, and publishCopy writes image: nil over the only reference (line 79). Later loads find a pubky.app profile, so they never migrate. Reproduced with the store source and a fake blob read that failed once: both loads returned no image and the Paykit copy's image was nil. Keep the copy's image until the avatar has moved, or retry the avatar on a later load. The load path now keeps the copy's image and retries (PubkyAppProfileStore.swift:73-93,222), which I confirmed by running it. A save in that window still loses it: save publishes the copy from a snapshot with no image (lines 128, 134), and after one name edit or tag add the copy's image was nil and a healthy reload never moved it, so keep the image until the move finishes.
  • A failed Paykit read makes the next save erase the profile tags (Bitkit/Managers/PubkyAppProfileStore.swift:74). load tolerates a failed Paykit read (line 53) and then builds the profile with tags: [] (line 74), so the next save overwrites the stored tags with nothing. Reproduced with the store source: after one failed read the load returned no tags while the copy held friend and work. A name edit then left the copy with none. Tags have no other store, so a failed read should not look like an empty list. tagsToKeep (lines 156-161) protects only the first save: publishProfileEdit commits the form's empty tags (PubkyProfileManager.swift:1022-1030) and the flag is cleared, so in my run the second save left the copy with no tags. A second failed read inside tagsToKeep publishes empty tags at once, which I also ran, so the PR's claim that a failed read no longer erases them does not hold.
  • Signed-out Create Profile no longer restores a Bitkit-only profile (Bitkit/Views/Profile/CreateProfileView.swift:186). With no session or stored key, CreateProfileView.remoteProfile calls fetchRemoteProfile, which now goes through store.load. That reads the Paykit profile but cannot return it, and migrate needs a signed-in session. Sign-out clears both keychain entries, so a user who signed out on master gets a blank form instead of the restore flow. Continue then publishes an empty bio, links and tags, and publishCopy replaces the Bitkit profile with them. The same base == nil branch writes bio: "" and links: []. If the signed-out lookup fails transiently for an identity that has a pubky.app profile, Continue clears that bio and those links at a fresh revision. Traced through the code, not run. load still returns the Bitkit profile only when signed in (lines 70, 76-81). Signed out it threw profileNotFound with a copy present and the blank Continue replaced the copy's bio, image and tags; after a failed lookup, Continue removed a pubky.app bio and links. The stored-key branch of remoteProfile, the read-only profile and Ring picker rows use the same load.
  • The name limit is not implemented and pubky.app limits only fail at Save (Bitkit/Components/ProfileNameField.swift:18). The PR says it limits names to 3 to 50 characters, but iOS has no such limit: ProfileNameField is unbounded and both profile views only reject an empty name. An out-of-range name, a bio over 160 characters, a sixth link, or a bio with a tab or pasted CRLF all reach the SDK and come back as an SDK error string in the toast. The \n replacement leaves \r behind for CRLF, which I checked by running it. On a new wallet, a two-letter name fails after signup, and completeIdentityCreation then discards the session. Bound these fields in the form so the limit is visible before Save. Names now stop at 3 to 50 and tabs and CRLF are handled, but a sixth link and over-long link titles or URLs still reach the SDK (ProfileEditFormView.swift:56 appends with no cap, and linksMaxCount only bounds the move). The forms count graphemes where Paykit counts scalars, so emoji or combining text inside the form's limit still fails at Save with an SDK string.
  • A pubky.app edit does not show in Bitkit until the next cold start (Bitkit/Views/Profile/ProfileView.swift:294). The PR says a pubky.app edit shows in Bitkit, but while the app stays running it only appears after a refused save. refreshProfile returns as soon as profile is set, and the other loadProfile callers are session restoration and the Edit Profile conflict path. Long-lived sessions therefore hold stale data, which also makes the retry overwrite above easier to hit. Refresh when Profile appears or on foreground. refreshProfile still returns once profile is set (ProfileView.swift:298). The other loadProfile callers are session restoration, the re-run after a failed save and the conflict paths, so a pubky.app edit stays unseen in a running session.
  • A profile deleted on pubky.app dead-ends at the load error (Bitkit/Managers/PubkyProfileManager.swift:1199). After a relaunch, a wallet-derived identity whose pubky.app profile was deleted can only sign out. load throws profileNotFound and loadProfile only logs it, so profile stays nil and Edit is unreachable, even though save could recreate the profile. Only the Ring refresh path treats not-found as profile setup. Traced through the code, not run. Route this case to profile setup or Edit Profile. load still throws profileNotFound for a known owner with no pubky.app profile (lines 69-70, 76-81), and loadProfile only logs it (PubkyProfileManager.swift:1237). Nothing outside the Ring refresh routes this to profile setup.
  • A failed avatar migration permanently removes the saved avatar (Bitkit/Managers/PubkyAppProfileStore.swift:158-160). An avatar upload failure permanently discards the legacy avatar reference. This catch returns the image-less profile as a completed migration. load then records the owner and overwrites the Paykit copy without its image. I reproduced one failed upload followed by a healthy reload. Both profiles stayed without an image, and the upload was never retried. Same as point 2: the load path now keeps and retries the avatar, but a name or tag save while the flag is set overwrites the copy's image with nil. I reproduced one failed fetch, one save and a healthy reload: both profiles ended with no image, so the avatar is still lost for good.
  • A failed profile-copy read causes existing tags to be erased (Bitkit/Managers/PubkyAppProfileStore.swift:52-54). A temporary Paykit-profile read failure can erase existing tags on the next edit. This catch turns the failure into nil, so the successful pubky.app load returns tags: []. That becomes the editor's tag list and is later written over the Paykit profile. I reproduced one failed read for a profile tagged vip, followed by a name-only save. The persisted tags became empty. Same as point 3: one failed read followed by two saves left the copy with no tags, and a second failed read inside the first save did the same. The manager's in-memory tags stay empty after the first save, so the fix has to reach publishProfileEdit too, or treat unread tags as unknown rather than empty.
  • First save after create or restore overwrites pubky.app edits (Bitkit/Managers/PubkyProfileManager.swift:765-776). The first save after Bitkit creates or restores a profile carries no revision, so it writes every field over whatever pubky.app holds, with no conflict check. createProfile commits a PubkyProfile with no revision (PubkyProfileManager.swift:765-776), and writeProfile at line 762 sends none. Nothing reloads it, because refreshProfile returns once profile is set. With baseRevision nil, save fetches the latest revision and calls update(from:base:nil) (PubkyAppProfileStore.swift:112-116), so name, bio and links are written at a fresh revision and can never be refused. 1eaabe4 caused this by removing bases, so the reply on the restore thread that the save diffs against what was shown no longer holds. The fallback at PubkyProfileManager.swift:905 has a related gap: a form opened with no revision can take a newer one than its data. Steps: create a profile in Bitkit, add a bio and a link on pubky.app, return without relaunching, add a tag. Confirmed by running the store source against a fake porting Paykit's revision check at d01659da. The tag add succeeded and removed the pubky.app bio and link. In the restore flow, Continue put the old bio back over the pubky.app edit. The manager steps are traced from the lines above, not run. Set the revision on the created profile, pass the restored profile's revision into the save, and refuse a save with no revision when a pubky.app profile exists. createProfile still commits a profile with no revision (PubkyProfileManager.swift:765-776), and save still writes every field at the latest revision (lines 112-116). I ran it: a tag add after a pubky.app edit removed the pubky.app bio and link.
  • Any save rewrites joined emoji and Persian text in untouched fields (Bitkit/Managers/PubkyAppProfileStore.swift:300-303). singleLine replaces joiners and other format characters with spaces, so any save rewrites and corrupts a name, bio or link title that holds them, even when the user did not touch that field. CharacterSet.controlCharacters covers Unicode Cf as well as Cc. Running it returned true for U+200D (zero width joiner), U+200C (zero width non-joiner), U+200E, U+00AD and the emoji tag characters. Paykit only rejects char::is_control, which is Cc, so none of these needed replacing. save normalises every field before diffing (PubkyAppProfileStore.swift:101), so the changed text differs from the base and is written. migrate applies the same function. The result breaks the technologist and rainbow flag emoji into separate symbols, and Persian text loses its non-joiner. Confirmed with the store source: a pubky.app bio holding those two emoji went through a name-only save and was stored with each joiner replaced by a space. A tag add sends the same fields, so it does the same. This breaks the claim that only changed fields are written. Test properties.generalCategory == .control instead. singleLine still tests CharacterSet.controlCharacters (lines 300-303). A name-only save rewrote an untouched bio, replacing U+200D and U+200C with spaces.
  • Free-text links are dropped or rewritten as mail and phone links (Bitkit/Managers/PubkyAppProfileStore.swift:311-325). linkUrl treats free-text link values as URLs, so migration drops some and rewrites others into wrong links, although ProfileLinkRow.destination documents link values as free text. Run through linkUrl: medium.com/@alice and youtube.com/@bitkit become mailto: URLs because they contain @, 2024-01-01 becomes tel:20240101, 1234 becomes tel:1234, and New York returns nil. migrate drops a nil link, and load then replaces the Paykit copy, so the value is gone from both profiles. On a save, the same value falls back to the raw text and Paykit refuses it as an invalid URI. The mailto: form is published on the public profile. Social links with @ are common, so check for a path before treating a value as an email address, and keep free text out of tel: unless it looks like a phone number. linkUrl is unchanged (lines 311-325). Running it, medium.com/@alice still becomes a mailto: URL, 2024-01-01 becomes tel:20240101 and New York returns nil.
  • A move finished by a save leaves the avatar on Paykit storage (Bitkit/Managers/PubkyAppProfileStore.swift:261-265). When a save finishes a move that load could not, as with a short legacy name, the avatar is not moved and pubky.app is given the Paykit URI. The save has no base, so update sends image: data.image (PubkyAppProfileStore.swift:264). Paykit's validate_image only checks that it is a URI. The save then marks the identity as owning a pubky.app profile without setting the pending flag, so no later load moves it. Confirmed with the store source: after saving a valid name, pubky.app's image was the Paykit path, with zero uploads, and a reload changed nothing. The PR says the avatar is re-uploaded as a pubky.app file so pubky.app shows it. In this same state, a tag add sends the short name and is refused by the SDK, which master allowed. update with no base still sends data.image (line 264) and save never sets the pending flag. A valid save after a short-name load put the Paykit URI on pubky.app with zero uploads, and a reload changed nothing.
  • A retried avatar move overwrites an avatar set on pubky.app (Bitkit/Managers/PubkyAppProfileStore.swift:73-75). The pending avatar retry replaces whatever image pubky.app holds, so an avatar the user chose there is overwritten by the old Bitkit one. Line 73 runs moveAvatar whenever the identity is pending and the copy has an image, without checking current.profile.image. The publish uses the fresh revision, so nothing refuses it. Confirmed with the store source: after one failed move, I set an image on pubky.app and loaded again, and pubky.app's image became the re-uploaded Bitkit avatar. This goes against the PR's claim that a pubky.app edit is never overwritten. Skip the move and clear the flag when pubky.app already has an image. Line 73 still retries without checking current.profile.image. An image set on pubky.app after a failed move was replaced by the re-uploaded Bitkit avatar on the next load.
  • A failed Paykit read makes Create Profile treat a profile as missing (Bitkit/Managers/PubkyAppProfileStore.swift:60-64). A failed Paykit read with no pubky.app profile now reaches Create Profile as profileNotFound, which is the case remoteProfile exists to prevent. load keeps going after the failed read, then throws profileNotFound at line 80 because there is no copy to move. CreateProfileView.swift:198 treats that as no profile, clears remoteLookupFailed, and shows a blank form whose Continue replaces the Bitkit profile. The comment at lines 187-188 says a failed lookup must not do this. Before 5d805c4 this branch used resolveRemoteProfile, which surfaced the failure. refreshReusedRingProfile reads the same error as a deleted profile and starts setup. The harness confirmed load throws profileNotFound after one failed read of an existing copy. The view steps are traced, not run. Both reads go to the same homeserver, so the window is narrow. load should rethrow the read error when there is no pubky.app profile. load still continues after the failed read and throws profileNotFound at line 80, which CreateProfileView.swift:198 treats as no profile. I ran load with one failed read and an existing copy and it threw profileNotFound, so the read error still needs rethrowing.
  • Profile save commits after an await with no session re-check (Bitkit/Managers/PubkyProfileManager.swift:1017-1032). publishProfileEdit checks the session at line 1017, suspends at line 1031 to read the revision, then commits, so a sign-out or identity switch in that gap commits the old identity's profile. The doc comment at lines 986-988 says an overtaken save changes nothing here. The gap is one actor hop, and I traced it rather than ran it. lastSavedRevision also returns the latest save for the key, so overlapping saves from Profile and Edit Profile can pair one save's fields with the other's revision. Returning the revision from the save, or re-checking after the await, closes the session gap; returning it closes both. publishProfileEdit still suspends on lastSavedRevision at PubkyProfileManager.swift:1031 after its last session check and commits with no re-check. The revision is still read by key rather than returned by the save, so overlapping saves can still pair the wrong fields and revision.
  • Edit Profile silently cuts a stored bio longer than 160 characters (Bitkit/Views/Profile/EditProfileView.swift:70-74). The new bio cap truncates a longer stored bio as the form loads, so any Save from Edit Profile rewrites it even if the user only changed the name. Paykit reads bios up to 4096 characters, and d01659da stopped such a bio from blocking a save. .onChange(of: bio) then cuts it when .task fills the form (EditProfileView.swift:70-74), the value differs from the base, and update.bio is written. pubky.app's own client stays within 160, so this needs another writer. Traced, not run. Apply the cap to user input, or skip it when the bio is unchanged from the loaded value. .onChange(of: bio) still truncates every assignment, including the one from loadProfileData (EditProfileView.swift:70-74), so a name-only save writes the cut bio. Traced, not run.
  • PR description and evidence predate the last two commits [one reviewer only]. The Out of Scope bullet still says Delete Profile leaves the pubky.app profile in place, which 5d805c4 changed. Evidence stops at 5003641 for unit tests and 08f6ce8 for device runs, and no QA case covers delete. These two commits add one test, for linkUrl, so the revision, pending avatar and tag paths have no coverage. The stale Out of Scope bullet is gone and unit evidence now names 1eaabe4, but no QA case covers delete and the device run is still on 08f6ce8. The revision, pending avatar and tag paths still have no tests, and those are where points 2, 3 and 11 reproduce.
  • New profiles overwrite external changes on their first edit (Bitkit/Managers/PubkyAppProfileStore.swift:112-116). Create a profile, change its bio on pubky.app, then save only its name in Bitkit before relaunching: the newer bio is erased. createProfile commits a profile without a revision (PubkyProfileManager.swift:765-776). This branch therefore fetches the latest revision and writes every stale field. I confirmed this using the current store source with revision-checked responses: the name save succeeded and removed the external bio. Same as point 11: the created profile has no revision, so the first edit goes through the nil-base branch at PubkyAppProfileStore.swift:112-116. My run lost the external bio this way.
  • A pending avatar move overwrites a newer pubky.app avatar (Bitkit/Managers/PubkyAppProfileStore.swift:73-74). After one failed legacy avatar move, setting a new avatar on pubky.app is undone by the next Bitkit profile load. This branch retries the legacy URI against the latest snapshot even when that snapshot already has an image. I confirmed the sequence with the current store source: a failed download, a remote avatar change, then a healthy load replaced the new image with the old avatar. Same as point 15: line 73 ignores an image already on pubky.app and moveAvatar publishes at the fresh revision, so the check cannot refuse it. Reproduced with a failed move, an external avatar change and a healthy load.

new since the last review

worth doing, does not block

  • Pending avatar retry uploads a pubky.app file record as the image (Bitkit/Managers/PubkyAppProfileStore.swift:206)
  • A profile deleted on pubky.app returns after a reinstall or wallet restore (Bitkit/Managers/PubkyAppProfileStore.swift:68-70)
  • A failed tag write keeps the text edit, then blames pubky.app on retry (Bitkit/Managers/PubkyAppProfileStore.swift:133-137)

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Tracked in #1287 for 3.2.0; this PR now closes it.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 718b20221 and the master merge a8b623d56: the MEDIUM is fixed, confirmed on device. Approving.

  • 718b20221: a save with unchanged tags writes pubky.app, and the Paykit copy follows as a tracked refresh in copyRefreshes. The next save waits for that refresh and a deletion refuses or waits for it. A tag change still waits for the copy, so a copy failure is still reported.
  • Merge a8b623d56: keeps Paykit rc7 over master's rc6 in project.pbxproj and Package.resolved. The other conflicts are test-file formatting.
Edit Profile Save → saved (iPhone 17 Pro, staging) run 1 run 2 run 3
this PR a8b623d56, name only 4.25 s 5.43 s 4.57 s
master fe0cc2de4 5.43 s 10.09 s 8.84 s
this PR, tag change 13.95 s 9.39 s —

The builds alternated, and every run started at load under 7. After a name-only save, /pub/paykit/profile.json had the new name and the tags about 4 s after the toast, and Profile shows both after a relaunch.

Device gate: a8b623d56 (perf re-run only); the journey and manual results from e016df03c stand, since this delta changes only the save's copy write — journeys: 1 passed; manual: 1, 3, 4, 5, 6 passed, 2 and 7 not run (need a pubky.app-side write with the identity key); perf: name-only Save 4.3–5.4 s (master 5.4–10.1 s).

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

@ovitrif
ovitrif requested a review from jvsena42 November 11, 2026 14:26
@ovitrif ovitrif removed this from the 3.2.0 milestone Nov 11, 2026
pwltr
pwltr previously requested changes Nov 11, 2026

@pwltr pwltr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 49f305a.

High — Bind automatic scope reauthentication to the originating session

Bitkit/Managers/PubkyAppProfileStore.swift:477-483 captures the active key after an Identity error and calls unconditional sign-in. If sign-out runs before that recovery acquires the SDK lock, recovery can subsequently restore the old identity's persisted credentials and authenticated SDK. This is reachable during legacy-profile migration from a profile load, while the cached-profile screen exposes Sign Out.

The manager's session revision rejects stale UI results, but recovery never checks it. The expected-identity guard protects only the write after reauthentication, when the old identity has already been restored. The deletion gate also does not cover ordinary sign-out.

Route scope recovery through a session-bound operation that validates ownership under the same lock before reauthentication. The interleaving is supported by the source trace; it was not reproduced dynamically.

Review details

Quick review
Model: Claude Opus 5.5
Reasoning effort: high

General review
Model: GPT-6.1 Sol
Reasoning effort: high

Combined review
Model: GPT-6.1 Sol
Reasoning effort: high

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of b269c39ae and 49f305a3f: no findings. Approving.

  • b269c39ae: a background copy write no longer clears tagsUnavailable, so it can't hide a failed read by a load that ran meanwhile.
  • 49f305a3f: a save that read the copy itself clears the flag only when tagsReadEpochs hasn't moved since it began, so a load that found the copy unreadable in between wins.

Neither changes the name-only Save path I timed at a8b623d56, so those timings stand. Device gate: not re-run for this delta.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr Fixed in 4b28bd5. Scope recovery now goes through PaykitSdkService.reauthenticate(expectedIdentity:), which runs as one operation under the SDK lock that sign-out and sign-in also take. Under that lock it checks that the identity the failed write was for is still signed in, and that the stored key is that identity's, before it signs in. A sign-out that lands first leaves no identity signed in, so recovery throws identityChanged and restores nothing. All five callers pass their identity. testReauthenticationSignsInOnlyWhileTheExpectedIdentityIsSignedInWithItsOwnKey covers signed out, another identity signed in, and a stored key of another identity (no sign-in attempted in each), plus the matching case, which does sign in.

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

16fd4d8 moves where the unread-tags flag is cleared, to match synonymdev/bitkit-android#1865 after @coreyphillips found that clearing it on the read, before a synchronous copy write, let a failed write lose tags there (thread). Here, a save now clears the flag after its own read only on the background path, where the stored profile keeps the copy's tags. The synchronous path clears it only on a successful write. iOS was already covered, because a failed write stores the profile with tagsUnread set and Profile passes that to its next save. This keeps the two apps the same.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 4b28bd578 and 16fd4d8f7: no findings. Approving.

  • 4b28bd578: the pubky.app-scope re-sign-in now goes through PaykitSdkService.reauthenticate(expectedIdentity:). Inside the same operationLock as the sign-in, it checks that expectedIdentity is still signed in and that the stored key derives to it, and throws identityChanged otherwise. So a sign-out or another identity's sign-in that lands during a profile write is never undone. Covered by the new PaykitSdkIdentityCheckTests cases.
  • 16fd4d8f7: a save no longer clears tagsUnavailable itself, which leaves it to the next load that reads the copy. That is the conservative side.

Neither touches the name-only Save path I timed at a8b623d56. Device gate: not re-run for this delta.

Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

@ovitrif

ovitrif commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

QA for 49f305a:

Since e016df0, a save with no tag change returns after the pubky.app write and lets the Paykit copy write follow in the background; master was merged in; and a save that read the copy itself now clears the unread-tags flag. So this run repeats the cases that save tags and reload, plus tag edits from Profile. Cases 3 to 6 passed at 308c7c8 (QA). The head since, 16fd4d8, adds 4b28bd5 (sign in again for the pubky.app scope only while the same identity is signed in) and 16fd4d8 (clear the unread-tags flag only where a failed copy write can't lose tags). Both change paths the UI can't stage: a session from before pubky.app writes, and a failed copy write.

Tested on an iPhone 17 Pro simulator (iOS 27), Debug build against staging, with Paykit 0.1.1-rc7. The stored profiles were read from the homeserver with the wallet's own key, and the "edit in pubky.app" was a write of profile.json with that key.

🟢 J1 and 1. Edit the name and a tag
pubky.app holds a profile with a bio, status away and an extra field. Renaming it to Edited In Bitkit and adding the tag friends updates pubky.app's name and keeps the bio, status and extra field; the Paykit copy has the new name and the tag. After a relaunch, both stored profiles are unchanged and Profile shows the name, bio and tag. The recording also shows a first save that hit "Your profile changed in pubky.app", because the test wrote pubky.app after the form was built; the form reloaded and the edit was made again.
case1-j12x.mp4

🟢 2. A profile changed in pubky.app during an edit
While Edit Profile is open, pubky.app gets a new bio and status busy. Saving a new name shows "Your profile changed in pubky.app", reloads the form with the new bio, and leaves pubky.app unchanged. Saving Renamed In Bitkit again stores the name and keeps the bio and status, and the copy has the new name and still the tag friends.
case2-conflict2x.mp4

🟢 7. A pubky.app edit shows when Profile opens again
With Profile opened earlier in the session and the tag friends on the profile, pubky.app gets a new bio. Going back to the wallet and opening Profile again shows the new bio and the tag, with no relaunch. The copy has the new bio and keeps the tag.
case7-refresh2x.mp4

🟢 8. Add and remove a tag from Profile
On the Profile screen, adding friends next to second stores both in the Paykit copy, and removing second leaves only friends, which Profile shows. pubky.app's name, bio and status stay unchanged.
case8-profile-tag2x.mp4

These runs keep the Paykit copy readable and its writes successful, so they show that saving and reloading tags did not regress. Save timing was measured by @jvsena42 at a8b623d; nothing since changes the name-only save path.

@pwltr pwltr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 16fd4d8, the one commit since 4b28bd5. No findings.

A save that read the copy itself now clears the tags-unread flag only on the background path, where the stored profile keeps those tags. A tag change clears it only after its copy write succeeds; a failed write keeps the flag, with the profile stored under its previous tags. The check now runs after waiting for earlier copy writes, but it still compares the read counter from the save's start, so a load that failed meanwhile keeps the flag set.

CI at 16fd4d8: SwiftFormat and validate passed; Run Tests, Run Integration Tests and build-local were still running.

Review details

General review
Model: Claude Opus 5.5
Reasoning effort: high

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Merging: approved by @jvsena42 and @pwltr on 16fd4d8, CI is green, the QA passed, and every thread is answered. It closes #1287 and #1243 and ships in 3.1.1 with its Android twins, synonymdev/bitkit-android#1801 and synonymdev/bitkit-android#1865.

@ovitrif
ovitrif merged commit a1dbb83 into master Nov 11, 2026
21 checks passed
@ovitrif
ovitrif deleted the feat/pubky-app-profile branch November 11, 2026 16:55
@piotr-iohk piotr-iohk mentioned this pull request Nov 12, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: use the pubky.app profile directly bug: profile tags erased by a form built from a failed copy read

5 participants