Repository navigation
Open the app with a saved key; check the account's key in the background - #3064
Conversation
A device set up through "Sign in with Builderlab" stopped at "Sign in / Continue without signing in" on every launch, even with a valid saved Builderlab session, and Sign in reopened the browser. A key on the device now opens the app, whatever the setup route, and the buzz.setup-route.v1 note is gone. Once the saved session is signed in, LinkedKeyCheck reads the account's linked key (the read Settings makes) with the app already open. A match, no linked key, no session or a failed read does nothing. Another key opens a dismissable prompt whose Replace is setup's existing replace step: sign this key out, restart, and sign in into Welcome back. The welcome screen leads with Sign in when a Builderlab session is signed in or needs reauthentication, instead of reading the note. A failed setup's Use a different account still returns to the landing with its key for that session only. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
Codex Review Summary: ✅ CompletedThis 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: a9fe6eacfc
ℹ️ 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".
| const returning = | ||
| keyLanding || | ||
| session?.status === "signed-in" || | ||
| session?.status === "reauthentication-required"; |
There was a problem hiding this comment.
Wait for session restoration before choosing the primary action
On a no-key launch with a saved Builderlab session, the identity can become missing while the federated session is still restoring; the launch screen is then dismissed and this expression treats the user as new, briefly making Create account primary. If clicked before restoration completes, start(true) carries the signup intent through firstRunCredential’s restoration wait and can open the account-creation flow for a returning user. Preserve the welcome screen’s previous durable returning signal or keep the landing hidden/disabled until session restoration resolves before selecting the action.
AGENTS.md reference: AGENTS.md:L147-L150
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Fixed in 37c9e84. While the saved Builderlab session is still restoring, Sign in and Create account are both disabled, so a click can't start a sign-up for a returning user. Once the session settles, the label is chosen from it. There's a test for it: "while the saved Builderlab session restores, the landing offers neither…".
While the Builderlab session is still restoring, Create account and Sign in are disabled, so a returning user can't start a sign-up before the session says which leads. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
… removes this key Replace removes this computer's key, so the prompt now says so, and tells the user to copy a key with no confirmed backup from Settings → Identity first. It reads the account's backup status on a mismatch: without a backup of the account's key, Welcome back could not restore it, so the prompt only says where that key is. Not now (or OK) is remembered for the pair of keys, so it asks again only when either changes. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
An OK on the no-backup prompt was remembered for the pair of keys alone, so after turning on Identity sync on the other device, Replace was never offered. The dismissal now also records whether Replace was offered, checked after the backup status is read. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
A failed backup-status read was taken as no backup, so the prompt claimed the account's key had none, and a Not now was asked again. Whether a backup exists is unknown then, so the check does nothing until the next session revision; only a real unavailable answer or a list without the account's key shows the no-backup prompt. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz Signed-off-by: Bradley Axen <baxen@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket, Mantis and Groot coverage.
Reviewed head 21888f64316b9032d946a78ac9df907048dfcda0 against base/merge-base 9cc2881d63bd8d28fddf56065fc36d8dcbc7741e.
Two P2 findings are attached inline: preserve the replacement target across restart, and preserve backup-consent dismissal while the mismatch prompt is shown.
Optional documentation clarification: docs/buzz-help.md could also explain the no-backup OK case and that a later backup can make the same key pair prompt again with Replace.
Source-only review. No tests, builds, app launches or native interaction checks were run; the PR’s reported checks at earlier revisions do not establish validation of this head. Native restart/recovery and rendered keyboard/focus behavior remain unverified. The declared post-signout primary-button label change is left to the stated product discussion.
| servers={servers} | ||
| /> | ||
| viewer={viewer} | ||
| onReplace={() => void setup.start([replaceKey])} |
There was a problem hiding this comment.
🤖 P2 — Preserve the checked replacement target across restart. This new caller can start from a saved native session for account A while the browser is currently signed into B. replaceKey records only a timestamp; startup removes the device key and saved FI sessions (src-tauri/src/lib.rs:908–911), then the restart starts ordinary browser sign-in with forceLogin false. Nothing carries found.linked into that sign-in or checks the returned account against it. If B has no linked key, the no-key branch at lines 520–526 creates and binds a new key to B; if B has a backup, setup enters B’s recovery instead of the A recovery that this prompt promised. Carry the expected linked key through the restart and reject a different/missing linked key before any create, bind or recovery step, offering account switching. Cover both unlinked-B and backed-up-B returns, plus the matching-A path.
There was a problem hiding this comment.
🤖 Fixed in 1a3e730. Replace now records the key it promised to restore in the restart's note (requestSignInAfterRestart(linked), stored as buzz.sign-in-after-restart.expected.v1, withdrawn with the note on a failed sign-out). The restart's sign-in picks it up with the note, and a step right after the account check fails setup when the account is linked to a different key or to none. That happens before the key creation, bind, legacy adoption, backup lookup or recovery steps run. The failure offers only Use a different account: Try again is hidden because the answer won't change, and returning to the landing drops the expectation. The setup lane's own Replace passes its checked key too.
Tests: unlinked B and backed-up B both reach the failure, with no identity_create, no bind and no recovery. Matching A rests on Welcome back. The existing Replace tests now also check that the key is saved, and withdrawn on a refused sign-out. Both new failure tests fail against 21888f6.
| setFound(undefined); | ||
| }; | ||
| const account = session?.email ?? "Your Builderlab account"; | ||
| return found.restorable ? ( |
There was a problem hiding this comment.
🤖 P2 — Keep backup-consent state alive while showing this prompt. Returning this dialog instead of children unmounts BackupConsent, whose dismissal is held in a local ref. On an independent community with no saved backup choice, close the backup prompt while this account read is pending, then dismiss the later mismatch with Not now or OK: BackupConsent remounts with dismissed = false and asks again in the same session. Native reaches no_consent before checking account binding (nsec_backup.rs:557–575), so this ordering is reachable. It breaks the existing close-until-sign-out/relaunch contract (BackupConsent.test.tsx:274, docs/identity.md:994–997). Keep the consent owner mounted and hold its presentation/decision while the mismatch is visible; add coverage for that delayed-read sequence.
There was a problem hiding this comment.
🤖 Fixed in 1a3e730. LinkedKeyCheck now always renders its children, as a render prop told whether it's asking. While the mismatch prompt shows, BackupConsent stays mounted and gets paused, which joins its existing launch/import hold, so its dismissed ref survives. Once the mismatch prompt closes, it decides again and stays closed if it was closed.
Test: the backup prompt opens, it's closed with Escape while the linked-key read is held, then the read resolves to another key and OK is clicked. The test waits for the post-dismiss nsec_backup_choice re-read, then asserts that no dialog shows and nothing was saved. It fails against 21888f6 with the "Sync your identity with Builderlab?" dialog back.
…ed behind the mismatch The sign-in after Replace's restart now has to reach the account linked to the key the prompt promised to restore; another account, or one with no key, fails setup before any key is made, linked or restored, offering Use a different account. The linked-key prompt keeps BackupConsent mounted and pauses it, so a consent prompt closed earlier stays closed. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
A sign-up is a new account, so it can never be the one Replace restarted for. Sign in and the timed-out wait's Continue keep the check. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source follow-up via Wes’s account, integrating independent Rocket, Mantis and Groot coverage.
Reviewed 01264c49df66d99242978eed8a417471d2409ece against base/merge-base 9cc2881d63bd8d28fddf56065fc36d8dcbc7741e, focused on the two patches added since reviewed head 21888f64316b9032d946a78ac9df907048dfcda0.
The backup-consent dismissal finding is addressed in source: the consent owner stays mounted and pauses while the mismatch is shown. The help-text clarification is also addressed. The direct replacement restart now checks the expected linked key before setup side effects, but one P2 remains in the Open Buzz continuation path, detailed inline.
Source-only: no tests, builds or app/native flows were executed. Added tests were inspected, and the author’s validation claims remain separate from this review. Native restart/recovery and rendered focus behavior remain unverified. Earlier coverage and the stated product discussion about the post-signout primary label are preserved.
| const run = requestedSignIn.current; | ||
| if (run && takeSignInRequest()) run(); | ||
| if (!run || !takeSignInRequest()) return; | ||
| expectedKey.current = takeExpectedKey(); |
There was a problem hiding this comment.
🤖 P2 — Retain the replacement target when Open Buzz resumes sign-in. The initial restart request consumes the note and sets the expected key, but a subsequent buzz://sign-in request has no note left: takeExpectedKey() returns undefined, and this assignment clears the active target. If the automatic sign-in is still pending or has timed out, the existing Open Buzz handler resumes it through start(false); after that login completes, signIn sees no expectation and omits the new account check. An unlinked browser account B can therefore still get a newly created/bound key, or another linked account can enter its recovery flow, instead of stopping for the account A that Replace selected. Keep the current expectation when a plain resume request supplies no new key; leave clearing to the explicit landing/account-choice paths. Add the pending-restart → requestSignIn() → mismatched-account sequence to the new tests, covering both unlinked and other-linked returns with no create/bind/recovery.
There was a problem hiding this comment.
🤖 Fixed in a862910. The Open Buzz handler now keeps the current expectation when takeExpectedKey() returns nothing, so a plain buzz://sign-in that resumes the pending or timed-out restart sign-in still checks the account. Only the landing (Use a different account) and Create account clear it.
Added tests for pending restart sign-in → requestSignIn() → an unlinked account, and the same with an account linked to another backed-up key. Both stop with the wrong-account error, with no bind, identity_create or nsec_restore_begin. Both fail on 01264c4. Identity and IdentitySettings tests: 618 pass, tsc clean.
A plain Open Buzz request names no key, so resuming the pending or timed-out restart sign-in must not clear the expectation; only the landing, Use a different account and Create account drop it. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source follow-up via Wes’s account, integrating independent Rocket, Mantis and Groot coverage.
Reviewed a86291068fb71ecaf4c7e0389d92c0dab20aa823 against base/merge-base 9cc2881d63bd8d28fddf56065fc36d8dcbc7741e, limited to the repair added since reviewed head 01264c49df66d99242978eed8a417471d2409ece; the eight earlier patches retain their previous coverage.
No new actionable findings. The remaining Open Buzz P2 is addressed in source: a plain resume request keeps Replace’s expected key, while explicit Back, account switching and Create account still clear it. The added tests cover a pending restart sign-in resumed into both unlinked and differently linked browser accounts, asserting the wrong-account stop and no create, bind or restore. The earlier backup-consent dismissal repair remains intact.
Source-only: no tests, builds or app/native flows were executed. Tests were inspected, not run; the author’s validation claims remain separate. Native restart/recovery and rendered focus behavior remain unverified. The stated product discussion about the post-signout primary label is unchanged.
* origin/main: deps: accept RUSTSEC-2026-0459 for libsqlite3-sys 0.38, move rmcp to 3.9.0 (#3077) fix(communities): accept invite links and bare hosts in Join a community (#2412) fix(builderlab): open a remote agent's Activity on the conversation it was opened from (#3051) refactor(builderlab): keep capabilities fresh in the session (#3003) feat(moderation): show invite links as a QR code (#3029) Keep Create agent's Model row readable and give Goose a Thinking row (#2673) Signed-off-by: Bradley Axen <baxen@squareup.com>
kazemek21
left a comment
There was a problem hiding this comment.
🤖 Kyle reviewed this PR and the Nexus review-tracker notes and approved with the non-blocking comment below.
Posted by Codex (gpt-6.1-sol) on behalf of kylez
| now = Date.now(), | ||
| ): () => void { | ||
| try { | ||
| localStorage.setItem(AFTER_RESTART, String(now)); |
There was a problem hiding this comment.
🤖 [P2] Persist the replacement target atomically
If writing the restart timestamp succeeds but writing EXPECTED throws, this catch leaves automatic sign-in armed without its expected-key guard. I reproduced this by throwing QuotaExceededError on the second write: takeSignInRequest() returns true while takeExpectedKey() returns undefined. After Replace, a different browser account can therefore proceed into key creation/linking or recovery. Could these values be stored atomically, or could a failed expected-key write remove the restart marker?
Posted by Codex (gpt-6.1-sol) on behalf of kylez
There was a problem hiding this comment.
🤖 Good catch, thanks. Fixed in 8d8330e: the restart note and its expected key now live in one localStorage item (buzz.sign-in-after-restart.v1, JSON { at, expected }), written with a single setItem, so the note can't be saved without its key. Withdrawing removes that one item. A bare timestamp from an earlier build still reads as a note that expects no key.
New test in sign-in-requests.test.ts reproduces your case: storage accepts one write and then throws QuotaExceededError. On the old code takeSignInRequest() is true and takeExpectedKey() is undefined, so the test fails. On the fix the key comes back with the note, and storage that refuses every write saves no note.
Writing the restart timestamp and the expected key as two localStorage items let a failed second write arm the sign-in after Replace without its account check. Both now live in one JSON item, written in one setItem; a bare timestamp from an earlier build still reads as a note expecting no key. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
…nt-progress-rows * origin/main: (53 commits) fix(search): make Command F search the Bestie conversation (#3084) fix(plugins): keep failed update checks on the plugin's Settings row instead of a toast (#3088) Show single-agent Stop on the activity bubble (#3140) fix: support explicit hosted browser origins (#3149) fix: keep Codex tool namespace descriptions bounded (#3128) fix(messages): show "New messages" in channels longer than one page (#3071) Keep shared agent guidance generic (#3134) fix(typing): a thread reply also ends the agent's channel-level typing (#3148) Stream protected evidence across agent runtimes (#2882) feat(identity): consolidate key and account controls (#3069) Add portrait-colored foil and animated agent card reveal (#3144) NEXUS-4190: Disclose unavailable saved Solo choices (#2544) docs: mark the plugin API as preview and say which parts are steady (#3129) Open the app with a saved key; check the account's key in the background (#3064) fix(federated-identity): name only communities that require NIP-FI as affected by a switch (#3072) feat(pairing): signed in with Builderlab, say to get the app instead of pairing (#3039) Show Add agent button on shared agent previews (#3131) NEXUS-4224: Preserve peer unread intent during automatic reads (#2641) fix(plugins): review a pinned link to an installed plugin's source as an update (#3024) perf(profiles): resume verified-profile batches after a planned renewal (#2293) ... Signed-off-by: Gil Birman <gil@squareup.com> # Conflicts: # docs/buzz-help.md
Problem
If a device was set up through Sign in with Builderlab, it stopped at Sign in / Continue without signing in on every launch. That happened even when a valid Builderlab session had restored from the Keychain, and Sign in opened the browser again. The cause is the
buzz.setup-route.v1note added in #2579.App startup doesn't need Builderlab. A key on the device is enough to open the app, and the Builderlab login only matters per community. The one real check the relaunch sign-in did was compare the account's linked key with this device's key, and the saved session can do that quietly.
Change
setup-route.tsand its call sites are removed. A device with a key opens straight to the app, however it was set up.LinkedKeyCheck). With the app open, once the saved session is signed in, it reads the account's linked key with the samefederatedBuilderlabAccount(...).linkedKey()call Settings → Identity makes. It does nothing if the keys match, the account has no linked key, there is no session or the session was refused, or the read fails. Launch never waits on it.replaceKeystep: sign this key out without erasing, restart, sign in again on its own, then take the no-key lane to Welcome back, which restores the account's key. No new recovery flow. The restart's note (buzz.sign-in-after-restart.v1) carries the account's linked key in the same stored item, written in one step, so a storage failure can't save the note without it. If the browser signs in to an account linked to another key, or to none, setup fails before any key is made, linked or restored, and offers only Use a different account. Create account drops the expectation, since a new account can't be the one Replace restarted for; Sign in, the timed-out wait's Continue, and an Open Buzz that resumes the restart's sign-in keep it."unavailable"), Replace isn't offered, because Welcome back couldn't restore the key. The prompt says the account's key lives on another device and offers only OK.buzz.linked-key-dismissed.v1), so it asks again only if either key changes, or once the account's key gets a backup after an OK.signed-inorreauthentication-required; otherwise Create account does. Both buttons are disabled while the saved session is stillrestoring, so the label never flips under a click and a returning user can't start a sign-up.Relay or server load: one kgoose read per client for each signed-in session revision, made through the existing native forwarder, plus a backup-status read only when there's a mismatch. There are no relay reads, polls or retries.
Tests
src/features/identity/identity.test.tsx:signed-inorreauthentication-requiredsession.src/features/identity/sign-in-requests.test.ts: the restart note's expected key is taken once, only while the note is fresh, and a withdrawn note leaves none. Storage that accepts one write and then refuses still saves the note with its key, storage that refuses every write saves no note, and an earlier build's bare-timestamp note reads as expecting no key.afterRefusedLink+leaveWithKey). Their assertions are unchanged, except "no key created" now means "only the key that first setup made."origin/main'sWelcome.tsxandsetup-route.tsrestored, all of the new behavior tests fail (checked for the first six at a9fe6ea; the wording, Replace-gating and remembered-pair tests assert copy and buttons the earlier dialog didn't have).Validation at 1a3e730 (01264c4 adds only the Create-account drop and its test, which fails on 1a3e730; identity + IdentitySettings 616 pass, full suite passed as pre-push): identity,
src/app/IdentitySettingsand deep-link tests pass (24 files, 708 tests), biome and tsc are clean, and the full suite passed as the pre-push hook. The three new review-fix tests fail against 21888f6'sWelcome.tsx,LinkedKeyCheck.tsxandBackupConsent.tsx.Earlier validation at 996458d: identity plus
src/app/IdentitySettingstests pass (610). The fullvitest runpassed as the pre-push hook, andpnpm checkpassed at b4f49ca; the later commits only touchLinkedKeyCheck.tsx, its tests and docs, and biome and tsc are clean on them. Earlier pre-push runs failed in unrelated files (relay, agents, app navigation) while other worktrees' suites loaded the machine, and passed on retry. CI passed on 1eb9b89. No Rust changed, socargo testwas not run.Deferred / not done
How to try it
localStorage.removeItem("buzz.linked-key-dismissed.v1")in the app's web inspector and relaunch.