Repository navigation
feat: use the pubky.app profile directly - #1801
Conversation
|
Pushed 3e57c25: a profile deleted on pubky.app after Bitkit saw it is no longer recreated from Bitkit's copy, the rebase onto master moves the new profile calls to the Pubky queue, and a test covers the one-time sign-in for sessions without the pubky.app scope. |
|
QA for a42bc35: Tested on a Pixel 10 Pro emulator (Android 16), dev 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 from staging Nexus, and the "edit in pubky.app" was a write of 🟢 Create and edit a profile with an avatar, Edit the pubky.app profileCreate and edit a profile with an avatar On a new wallet, a profile named Android Fresh with an avatar is saved to pubky.app, and staging Nexus indexes the name, the avatar file and its image. Renaming it to Android Edited updates pubky.app and keeps the avatar.
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 BitkitUpgrade a wallet whose profile is only in Bitkit A master build saves Android Master Profile only to Bitkit's Paykit copy, and pubky.app has no profile. After updating to this build, opening Profile signs in once with the pubky.app access and moves the profile to pubky.app. A rename then saves to both pubky.app and the Paykit copy.
🟢 Upgrade a wallet that already has a pubky.app profileUpgrade a wallet that already has a pubky.app profile A master build saves Android Bitkit Only, then pubky.app gets a profile named Android 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.
🟢 A profile changed in pubky.app during an editA profile changed in pubky.app during an edit While Edit Profile is open, pubky.app gets a new bio and status. 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.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a42bc35292
ℹ️ 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".
[High impact] The change reaches Bitkit’s public profile reads, writes, and sign-in permissions. Do not merge until saves and migration preserve existing profile data and deleted profiles stay deleted. Findings
SummaryBitkit now reads and edits the user's pubky.app profile directly.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Load own profile] --> B[Read Paykit copy and tags]
B --> C[Read pubky.app snapshot]
C --> D{Profile exists?}
D -->|Yes| E[Use pubky.app fields]
D -->|No and not previously seen| F[Migrate Paykit profile and avatar]
E --> G[Refresh Paykit copy]
F --> G
H[Save editor fields] --> I[Build changed-field update]
I --> J[Publish at expected revision]
J -->|Conflict| K[Reload and ask user to edit again]
J -->|Success| G
Reviews (1) · Last reviewed commit: "fix: save a multi-line bio as one line" · Reviewed by Greptile |
talosmachina
left a comment
There was a problem hiding this comment.
1 finding (1 P1). Moves the own profile to pubky.app: reads and writes go through the SDK with revision checks, Bitkit-only profiles are migrated on load, and the Paykit copy follows. Reviewed a42bc35, full tier, static: on the Bitkit track an auto review does not build the branch.
What I checked, and 3 candidates I ruled out
Read in full: PubkyRepo.kt profile region (load, save, migrate, scope refresh), EditProfileViewModel.kt, PaykitSdkService.kt capability changes, and the SDK side in pubky/paykit-rs#270 (profiles.rs, pubky_app.rs, contacts/mod.rs validation).
CI: build-and-test and compose-ui fail resolving com.synonym:paykit-android:0.1.1-rc7, which is not published while pubky/paykit-rs#270 is open.
Ruled out
- Scope refresh loop:
withPubkyAppScopesigns in again once on an identity error and rethrows after. - Save after a load that raced it:
continueSignInkeeps the current sign-in when the key is unchanged. - Edited multi-line bio: the save path replaces
\nbefore writing.
Merge confidence: 1/5, the pinned Paykit artifact is unpublished so the branch does not build, and the P1 above.
|
Pushed ff8a1e1, 2f4f135, b17caec and 077b8b0 for the review threads: Delete Profile also deletes the pubky.app profile of a wallet-owned identity; a failed avatar move keeps the original and retries; migration flattens line breaks, truncates by code points, keeps scheme-less links as full URLs and leaves the Bitkit profile usable while it cannot finish; a failed Paykit read no longer erases tags and a failed tag write is reported; profile writes check the identity they were started for; the migration history survives sign-out; Edit Profile saves against the profile and revision it opened with, while refreshes keep the repository pair current. |
|
QA for 409d744: Tested on a Pixel 10 Pro emulator (Android 16): dev build against staging, with a local Paykit build of pubky/paykit-rs#270 from before d01659da. That commit only limits validation to the fields a save writes, and no case here had stored fields over pubky.app's limits. The upgrade cases started from a master build (ebe9bc0). Where a test needs pubky.app, the stored profile was read from the homeserver and from staging Nexus. The "edit in pubky.app" step was a write of 🟢 J1, 1 and 5: create a profile with an avatar, edit it, relaunchOn a new wallet, a profile named Android R2 with an avatar is saved to pubky.app, and staging Nexus indexes the name and the avatar file with its blob. Edit Profile shows the pubky.app note. Renaming the profile to Android R2 Edited updates pubky.app and keeps the avatar, and the name survives a relaunch.
🟢 2: a profile changed in pubky.app during an editWhile Edit Profile is open, pubky.app gets a new bio and status. 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.
🟢 3: upgrade a wallet whose profile is only in BitkitA master build saves Android R2 Master with an avatar only to the Bitkit copy, and pubky.app has no profile. After updating to this build, opening Profile moves the profile to pubky.app. The avatar is re-uploaded from Paykit storage as a pubky.app file, and the Bitkit copy now points at that file.
🟢 4: upgrade a wallet that already has a pubky.app profileA master build saves Android R2 Bitkit Only, then pubky.app gets Android R2 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.
🟢 6: Delete ProfileEdit Profile → Delete Profile → Yes, Delete removes the pubky.app profile and the Bitkit copy; both now return 404. Bitkit returns to the profile choice screen.
|
|
QA for bda1793: Tested on a Pixel 10 Pro emulator (Android 16): dev build against staging, with Paykit 0.1.1-rc7. The upgrade cases started from a master build (ac1796a). Where a test needs pubky.app, the stored profile was read from the homeserver and from staging Nexus. The "edit in pubky.app" step was a write of 🟢 J1, 1 and 5: edit the name and the avatar, relaunchOn a new wallet with a profile named Android 1801, pubky.app gets status busy. Edit Profile shows the pubky.app note. Renaming the profile to Edited In Bitkit shows "Profile saved" and updates pubky.app and the Bitkit copy, with status busy kept, and the name survives a relaunch. Picking a new avatar saves it to pubky.app as a file with its blob, keeps the name and status, and staging Nexus indexes the avatar.
🟢 2: a profile changed in pubky.app during an editWhile Edit Profile is open, pubky.app gets a new bio and status. 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, status and avatar.
🟢 3: upgrade a wallet whose profile is only in BitkitA master build saves Android 1801 Master with a bio, a Website link `bitkit.to` and an avatar only to the Bitkit copy, and pubky.app has no profile. After updating to this build, opening Profile moves the profile to pubky.app with the same name, bio and link, now `https://bitkit.to`. The avatar is re-uploaded as a pubky.app file with its blob, the Bitkit copy now points at that file, and staging Nexus shows the profile.
🟢 4: upgrade a wallet that already has a pubky.app profileA master build saves Android 1801 Bitkit Only, then pubky.app gets Android 1801 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.
🟢 6: Delete ProfileEdit Profile → Delete Profile → Yes, Delete removes the pubky.app profile and the Bitkit copy; both still return 404 ten seconds later, and staging Nexus no longer finds the user. Bitkit returns to the profile choice screen.
|
|
QA for 521c3be: Tested on a Pixel 10 Pro emulator (Android 16): dev build against staging, with Paykit 0.1.1-rc7. The upgrade cases started from a master build (ac1796a). Where a test needs pubky.app, the stored profile was read from the homeserver and from staging Nexus. The "edit in pubky.app" step was a write of 🟢 J1, 1 and 5: edit the name and the avatar, relaunchOn a new wallet with a profile named Android 1801, pubky.app gets status busy. Edit Profile shows the pubky.app note. Renaming the profile to Edited In Bitkit shows "Profile saved" and updates pubky.app and the Bitkit copy, with status busy kept, and the name survives a relaunch. Picking a new avatar saves it to pubky.app as a file with its blob, keeps the name and status, and staging Nexus indexes the avatar.
🟢 2: a profile changed in pubky.app during an editWhile Edit Profile is open, pubky.app gets a new bio and status. 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, status and avatar.
🟢 3: upgrade a wallet whose profile is only in BitkitA master build saves Android 1801 Master with a bio, a Website link `bitkit.to` and an avatar only to the Bitkit copy, and pubky.app has no profile. After updating to this build, opening Profile moves the profile to pubky.app with the same name, bio and link, now `https://bitkit.to`. The avatar is re-uploaded as a pubky.app file with its blob, the Bitkit copy now points at that file, and staging Nexus shows the profile.
🟢 4: upgrade a wallet that already has a pubky.app profileA master build saves Android 1801 Bitkit Only, then pubky.app gets Android 1801 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.
🟢 6: Delete ProfileEdit Profile → Delete Profile → Yes, Delete removes the pubky.app profile and the Bitkit copy; both still return 404 ten seconds later, and staging Nexus no longer finds the user. Bitkit returns to the profile choice screen.
|
jvsena42
left a comment
There was a problem hiding this comment.
Delta review of the four commits since 73553d409 (head 521c3be2b): no findings.
- Save epoch (
6fd67b6b1): the epoch check runs insideprofileWrite(generation)(PubkyRepo.kt:1526-1530), so the deletion generation check still runs first. A save's own copy write takesprofileWriteMutexafter the load's check, and tag saves are serialized bytagUpdateMutex, so a load cannot publish the old tags over a running save. Thetry/finallymoves the epoch on when a save throws or is cancelled. - Marker after a failed copy read (
bda1793f2,:1381-1389): I walked pubky.app present/absent × copy read ok/failed × copy avatar on Paykit storage/elsewhere/missing. The marker is cleared only when pubky.app has an image, or the copy was read and holds no Paykit-storage avatar. A failed read keeps it, skips the move and writes no copy. - Marker before the create (
521c3be2b): a create that landed with a lost response moves the avatar on the next load; one that did not land migrates again. A save without a base keeps or clears the marker correctly (:1306-1320). - Upgrade from v3.1.0: no new persisted field or format; the delta only changes when the existing marker is written.
- perf: no new polling, timers or retries; one extra DataStore write per migration attempt.
CI is green at this head.
Device gate (partial): 521c3be2b — not run yet; J1 edit-pubky-app-profile.xml and manual tests 1-6 are next, now that paykit-android 0.1.1-rc7 is published. I'll post the result in a follow-up review.
Reviewer model: Claude Fable 5.2 (claude-fable-5-2), Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed
pwltr
left a comment
There was a problem hiding this comment.
Reviewed 521c3be.
Medium — Preserve the editor's tag-read provenance. In app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1347, preservation depends on the latest repository-wide tagsUnavailable flag. After a failed Paykit read, the displayed profile has empty tags. The user can open Edit during a later refresh; that refresh can recover the stored tags and clear the flag while the editor retains its empty list. Because those tags live only in Paykit, the captured pubky.app revision can still match. Saving an unrelated name change then successfully overwrites the unseen tags with an empty list. Capture tag-read availability alongside ProfileEditBase and use that captured provenance when deciding whether to replace or merge stored tags.
Low candidate — Overlapping saves defeat the epoch parity assumption. In app/src/main/java/to/bitkit/repositories/PubkyRepo.kt:1528, two active saves make profileSaveEpoch even again, allowing a load captured during them to pass the guard. The mutex covers individual writes rather than entire saves. Persistent stale-tag loss would additionally require a narrow interleaving around copy-write completion and the final epoch increment, so this remains a candidate. Track active saves explicitly or serialize whole saves before relying on parity.
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
A save merges the stored tags when its edit began while the Paykit copy was unread, not only when it is unread now, and a load refreshes the copy only while no save runs.
Fixed in cb4d9cd.
Fixed in cb4d9cd. A save now also counts itself in Full unit suite: 5028 tests, 0 failures; detekt clean. |
|
@ovitrif this conflicts with |
# Conflicts: # gradle/libs.versions.toml
|
@jvsena42 merged |
|
QA for 7296e0f: Since 521c3be, cb4d9cd only changes how a save keeps the tags in Bitkit's Paykit copy, and 7296e0f merges 🟢 J1 and 1. Edit the name and a tagpubky.app gets status busy. Edit Profile shows the pubky.app note. Renaming the profile to Edited In Bitkit and adding the tag friends shows "Profile saved", updates pubky.app's name and keeps the status; the Paykit copy has the name and the tag. After a relaunch, both stored profiles are unchanged and the name and tag show.
🟢 2. A profile changed in pubky.app during an editWhile Edit Profile is open, pubky.app gets a new bio and status away. 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, status and tag.
🟢 Tags from ProfileOn the Profile screen, adding family stores both tags in the Paykit copy, and removing friends leaves only family, which still shows after a relaunch. pubky.app stays unchanged throughout.
These runs keep the Paykit copy readable, so they show that saving and reloading tags did not regress. An edit that began while the copy could not be read is covered by |
jvsena42
left a comment
There was a problem hiding this comment.
Device gate at 521c3be2b. This is not an approval, because the branch now conflicts with master. cb4d9cdf4 (keep the stored tags when an edit began without them) was not built; I read it and it is clean. Setup: Gate_Throwaway emulator, staging homeserver. I read remote profiles over GET https://homeserver.staging.pubky.app/pub/... with the pubky-host header.
| Case | Result |
|---|---|
J1 edit-pubky-app-profile.xml |
Pass, all 8 actions. The footer note names the public pubky.app profile, "Profile saved" shows (screenshot, since toasts are not in android layout), and ProfileViewName shows the edit after a relaunch. |
| 1 | Pass. pubky.app's name changed, and bio, image and links were untouched. There was no status field to check. |
| 2 | |
3 (upgrade from master ac1796a85) |
Pass. Name (trimmed), bio, link and avatar moved to pubky.app. The avatar is byte-identical (sha1 bbe0fdc7…), and tags were kept. |
| 4 | Pass, via PR → master → PR: the PR showed the pubky.app values and rewrote the Bitkit copy, keeping tags. The pubky.app profile was created by this build, not by pubky.app. |
| 5 | Pass. The new avatar is on pubky.app (400×400 JPEG). |
| 6 | Pass. Both profile.json files return 404, and Nexus reports deleted:true. |
| Tag change right after an Edit save | Pass: [alpha, beta] after a relaunch. cb4d9cdf4 touches this path and was not re-run. |
| Force-stop between the create and the avatar move | Pass. The next launch moved the avatar. |
perf (tap to settled, master ac1796a85 vs this PR, same emulator):
- Open Profile after a relaunch: 3.80 / 2.93 / 2.88 s vs 1.82 / 2.09 / 3.01 s (plus one 15.5 s outlier at host load 70).
- Edit Save → Profile: median 8.8 s (6.8–13.3) vs median 13.2 s (9.4–24.7). The PR looks about 4 s slower, but host load swung between 5 and 150 during these runs, so I'm treating it as unverified. The save path logs no start or end line. The slow-save window has no WARN or ERROR. Homeserver lookups during a save were 2.6/s vs about 2/s on master. A rerun on a quiet host would settle it.
- Idle on Home, minutes 2–5: about 71 homeserver lookups/min vs about 75/min on master. No profile requests on either build.
Both builds log repeated acquire Pubky write lock (HTTP 405) warnings from Backup and the Paykit request refresh. That is the same on master, so not a finding here.
Device gate: 521c3be2b (head cb4d9cdf4, app code not rebuilt) — journeys: 1 passed; manual: 1, 3, 4, 5, 6 passed, 2 not run; perf: Profile open 1.8–3.0 s (master 2.9–3.8 s), Edit Save median 13.2 s (master 8.8 s, unverified under load); iOS: synonymdev/bitkit-ios#1235 gate running now.
I'll re-review once it's rebased.
Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed
jvsena42
left a comment
There was a problem hiding this comment.
Approving at 7296e0fc1. The device gate results for 521c3be2b are in my previous review.
cb4d9cdf4: clean.ProfileEditBase.tagsUnreadrecords that the Paykit copy was unread when the edit began, so the save merges the stored tags even if a later load read them.profileSavesRunningkeeps a load from refreshing the copy while any save runs, which matches the iOS fix in synonymdev/bitkit-ios#1235.- Merge
7296e0fc1: the only conflict isgradle/libs.versions.toml, resolved topaykitrc7 next to master's newbreez-sdk-spark. The rest of master (Spark) does not touch the profile paths the gate drove.
The tag-after-save check passed on 521c3be2b and was not re-run on cb4d9cdf4. The Edit Save timing (median 13.2 s vs 8.8 s on master) stays unverified: it was measured while host load swung between 5 and 150.
Device gate: 521c3be2b (head 7296e0fc1; cb4d9cdf4 and the master merge were reviewed, not driven) — journeys: 1 passed; manual: 5 passed, 1 not run (needs a second writer with the identity key); perf: see the previous review.
Reviewer model: Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed
|
Merged: approved by @jvsena42 at 7296e0f, checks green, device run of J1 and the tag cases on this head in #1801 (comment), and every review thread answered. @pwltr's review of 521c3be is addressed in cb4d9cd. The Edit Save timing difference in @jvsena42's gate was measured under heavy host load and is unverified. |
|
Tracked in #1864 for 3.2.0, closed now that this PR is merged. |
Closes #1864
Twin: synonymdev/bitkit-ios#1235
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
https:,mailto:ortel: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.Out of Scope
/pub/paykit/profile.json), which Bitkit keeps writing in line with pubky.app for other Paykit apps.Design
N/A — no design available.
Preview
Recordings are in the device-test comment: #1801 (comment)
QA Notes
Setup
Journeys
edit-pubky-app-profile.xml— the Edit Profile note names the public pubky.app profile, and a name edit saves and survives a relaunchManual Tests
/pub/paykit/profile.jsonis gone — pubky.app in a browser not in CapabilitiesAutomated Checks
PubkyRepoTest.kt— a save writes only the changed fields at the loaded revision, a profile changed in pubky.app is reported, a Bitkit-only profile moves to pubky.app, and a session without the pubky.app scope signs in again once; a restore saves against its pubky.app revision and a create never overwrites one, an unsaved tag change is reported with the revision to retry from, a pending avatar move yields to an avatar set on pubky.app, free-text links are not made links, a load running alongside a save does not write the old tags back, a pending avatar move survives a failed copy read, and a move records the pending avatar before it creates the pubky.app profilePubkyRepoTest.ktandContactSaveSessionChangeTest.kt— own profile loads read pubky.app with its revision0.1.1-rc3to0.1.1-rc6record fieldsEvidence
Gaps
Models used
claude-opus-5-5(reasoning:Not exposed)claude-opus-5-5(reasoning:Not exposed)Metadata note: the session reports its model but not a reasoning setting.