Repository navigation
Updated onboarding - #2183
Updated onboarding#2183
Conversation
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 261d1a4368
ℹ️ 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".
delkc
left a comment
delkc
left a comment
There was a problem hiding this comment.
🤖 Posting the copy and interaction suggestions Clay and I worked through in a disconnected local preview. The inline comments distinguish wording changes from proposed flow/consent changes. The shortened backup explanation still needs recovery-authority/cross-use disclosure review before shipping. Returning-account and community-consent copy are excluded because we did not settle changes to those screens. No code changes are being pushed with this review.
Offer explicit backup consent after key creation and replacement, persist declined backup, simplify recovery states, and require a name to complete onboarding joins. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Preserve onboarding membership-discovery coverage alongside the new Builderlab request and pending-login tests. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4185c2e7ec
ℹ️ 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".
| } | ||
| return; | ||
| } | ||
| if (launching) return; |
There was a problem hiding this comment.
When the launch overlay remains active for more than eight seconds, onboarding-intro-init.js deliberately clears data-welcome-intro and shows the normal window, but this hook has already captured phase === "waiting". Once launching becomes false, it proceeds without rechecking that the pending marker still exists, resizing an already-visible window and running the intro after the fallback fired. Revalidate the pending marker when the launch hold ends and transition to done if the bootstrap timeout has released it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29386f7a2f
ℹ️ 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".
| <OnboardingBack | ||
| onClick={onBack} | ||
| disabled={!!busy} | ||
| aria-label={backLabel} | ||
| /> |
There was a problem hiding this comment.
When backup fails, Failure receives backLabel="Continue without backup", but this change exposes that text only as the accessible label of a Back-arrow icon. Sighted users therefore see an action that appears to navigate backward even though activating it disables automatic backup and advances setup, making the only escape from a failed backup misleading and difficult to discover. Preserve a visible text button whenever backLabel describes an action rather than ordinary Back navigation.
AGENTS.md reference: AGENTS.md:L53-L56
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl (automated reviewer, via Wes's account): obsolete at b9a559d6, because Continue without backup was removed and no caller passes backLabel. The prop is now dead in Failure and can be dropped. The live consequence of removing the skip is the dead-end thread on Welcome.tsx:952.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 111bf4ef97
ℹ️ 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".
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9247f96cfd
ℹ️ 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".
| onBack={async () => { | ||
| setProfileStep(undefined); | ||
| setFinding(undefined); | ||
| }} |
There was a problem hiding this comment.
When decide() creates this profile step for a restored destination, profileStep.admission is absent but the membership has already been saved and selected. Clicking Go back now only clears the profile and destination state; because setup remains done, the user is sent to the new-community form instead of entering the successfully restored community. Preserve the restored-profile branch that calls leave(), while returning new joins to community selection.
AGENTS.md reference: AGENTS.md:L90-L93
Useful? React with 👍 / 👎.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1d9a161fb
ℹ️ 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".
| onClick={() => { | ||
| if (auth.status === "pending") backToLanding(); | ||
| else void signIn(); | ||
| if (login.snapshot().status === "pending") { | ||
| cancels.current++; | ||
| login.cancel(); | ||
| } | ||
| void signIn(); | ||
| }} |
There was a problem hiding this comment.
While the OAuth session is pending, this button remains labeled Sign in with email and clicking it cancels the active attempt only to immediately start another one. This removes the visible pending status and dedicated cancellation path required by docs/identity.md:36-40; users returning from the browser cannot tell that sign-in is still active and can accidentally launch repeated attempts. Render the pending/cancel state instead of restarting sign-in on the same action.
AGENTS.md reference: AGENTS.md:L90-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl (automated reviewer, via Wes's account): confirmed at b9a559d6. The removal looks deliberate, since identity.test.tsx:1933-1937 asserts the pending copy is absent. However, docs/identity.md:45-50 still promises Cancel sign-in. The test at identity.test.tsx:1404 is titled Cancel sign-in but actually aborts through Other options. Either restore a visible pending state with Cancel, or record the decision and update the docs and test title.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, with independent Rocket, Mantis and Groot lanes. Seven actionable findings (six P2, one P3) are inline; three corroborate existing review threads. The signed-in email repair is present.
Reviewed head e1d9a161fbe904b7c7c6f23bd8f0cf3ada7e1e78 against base 3e94f2ab417aa023abda4729c293ea0558d7836d. Source-only: no tests, builds or app execution. The public description and all 36 attached images were inspected. Hosted CI was still running at the single snapshot; Windows validation was skipped. Native auth/recovery/window behavior, rendered focus/accessibility, performance and human acceptance remain unverified. These are non-blocking review comments, not approval or merge authorization.
Apply Matt's onboarding feedback: automatic Builderlab backup with retry, direct restore completion, existing-profile preservation, and forward-only completion after admission. Refine onboarding copy and document the flows. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3ece7a5dd
ℹ️ 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".
| currentAccount = account(credential, identity); | ||
| reads.current = currentAccount; |
There was a problem hiding this comment.
When backup retries after a signed_out failure, this replaces reads.current, but the remaining findCommunities step was already created by finish() with the original credential-bound account. After the refreshed backup succeeds, that step therefore uses the expired credential; its rejection is deliberately swallowed by findCommunities, so the onboarding community picker silently loses the account’s owned communities. Construct the account-dependent finishing steps only after the credential refresh, or otherwise make them read the current account when they execute.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl (automated reviewer, via Wes's account): confirmed at b9a559d6. A focused journey probe ran a linked-key import → backup signed_out → same-account reauth → successful backup. The owned-community lookup then used the expired account (old reads=1, refreshed=0), and the swallowed rejection hides owned communities. Resolve reads.current when the finishing step runs, not when finish() builds the steps.
Preserve native federated first-run sign-in and its regression coverage alongside automatic backup and direct restoration. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9a559d6c9
ℹ️ 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 change = (next: string) => { | ||
| setText(next); | ||
| setFailure(undefined); |
There was a problem hiding this comment.
When a user reveals one key, clears the field, and then types, pastes, or loads another key, change now leaves revealed set, so the replacement nsec is displayed as plain text without a new Reveal action. Fresh evidence beyond the earlier masking report is this commit's deletion of the prior empty-value reset; restore that reset (or otherwise retire reveal state when replacing the secret) to preserve the sensitive-data contract in docs/identity.md:577-579.
AGENTS.md reference: AGENTS.md:L90-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl (automated reviewer, via Wes's account): reproduced at b9a559d6. Reveal → clear → paste leaves the replacement nsec as type=text, and resetting revealed on clear or replace fixes it in a probe. I'm noting it because this lane is for a private key. Whether to fix it follows the earlier deferral decision.
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account. This is a consolidated Court review (identity, native/community and UX lanes, plus an integration pass) at b9a559d6.
Verdict: two blockers before merge.
- First run dead-ends on deployments without backup.
- The recovery-authority disclosure is missing at the create/replace consent point.
The other new findings are P2/P3 and are explained inline. Per this repo's agent-review rule these are comments; humans decide what must be fixed.
Existing threads: I replied on the open Codex threads we confirmed or found obsolete. The intro timeout, close-during-intro reopen and first-run consent-origin concerns still reproduce in source, but they match Star Lord threads Kenny explicitly deferred, so they are not re-raised. The key-restore retry guard (Codex KeyRestore:35) is fixed at this head.
Validation: source review across all lanes. Two focused local journey probes reproduced the stale-account and re-mask findings, and both passed with minimal temporary corrections. Hosted CI is green; Windows was skipped. Not exercised: live hosted sign-in/backup, a deployment without backup, a packaged fresh install and the macOS window lifecycle. The PR body still shows the deleted optional backup screen, and its test counts predate this head.
Exit criteria: the two blockers are resolved, or the product owner explicitly records a decision on each.
| @@ -762,31 +904,30 @@ export function Welcome({ | |||
| read={profileStep.read} | |||
| defaultName={auth.account?.email.split("@")[0] ?? ""} | |||
| onContinue={(next) => finishJoin(profileStep, next)} | |||
There was a problem hiding this comment.
🤖 P2: the profile step has no exit. The Go back handler is gone, and ProfileStep offers only Continue. If the relay denies the publish, ProfileStep.tsx:112-113 tells the user to ask an administrator, while Continue keeps failing. Quitting is the only way out.
The earlier thread that asked for a safe back/cancel route when access is refused was answered as implemented in 4185c2e. e8eabdc then removed it (forward-only completion), and the reply was left in place.
Fix: when the publish is denied, offer Go back to community selection, or leave the join, while keeping forward-only completion for admitted, healthy relays.
| --border-floating-control: var(--border-prominent); | ||
| --affordance-selected: var(--neutral-3); | ||
| --affordance-disabled: var(--neutral-3); | ||
| --affordance-disabled: var(--neutral-4); |
There was a problem hiding this comment.
🤖 P2: this changes the disabled fill across the whole app, not just onboarding. --affordance-disabled moves from neutral-3 to neutral-4 in light and neutral-3 to neutral-5 in dark. That restyles every disabled Button, Input and Textarea (forms.css:152,231,494, workflows.css:355). With components.css:673, every disabled prominent button app-wide also gains an inset border. tokens/registry.ts:1136 still documents neutral-3 / neutral-3.
Fix: scope the onboarding treatment to the setup surface, or treat it as a deliberate system change and update the registry and DESIGN.md to match (see MAINTAINING_DESIGN_SYSTEM.md).
| --size-resize-seam: 0.875rem; | ||
| --radius-panel: var(--corner-panel); | ||
| --radius-control: var(--corner-control); | ||
| --radius-card: 2rem; |
There was a problem hiding this comment.
🤖 P3: --radius-card collides with an existing token. This unlayered declaration overrides the layered --radius-card: var(--radius-panel) in src/shared/styles/tokens.css:44. It moves the existing consumers to 2rem: globals.css:190, Messages.module.css:1248,1305, AgentCardAttachment.module.css:10 and Channels.module.css:758. Fix: give the onboarding card radius its own name.
| const content = ( | ||
| <> | ||
| <div className={styles.brand}>{intro}</div> | ||
| <div className={styles.actions} inert={introPhase !== "done"}> |
There was a problem hiding this comment.
🤖 P3: the intro blocks actions for about 8.4 seconds when no key is pressed. inert holds until finish, and only keydown finishes early (useWelcomeIntro.ts:93). Pointer and VoiceOver users wait out the animation. DESIGN.md says decorative motion never blocks an action. Fix: finish on pointerdown too. useWelcomeIntro has no test of inert/skip/reduced-motion behavior, and one focused test would cover it.
| <div data-onboarding-actions=""> | ||
| <OnboardingBack | ||
| aria-label={!failed && !saving ? "Cancel" : "Back"} | ||
| disabled={saving || canceling} |
There was a problem hiding this comment.
🤖 P3: an icon-only arrow carries Cancel. While native recovery waits, this back arrow is labelled Cancel and cancels the attempt. RestoreIdentity.tsx:124-127 does the same. Sighted users see navigation, not cancellation, and OnboardingBack has no visible hint (DESIGN.md:325). The earlier copy review asked to keep a visible Cancel during the browser wait. Fix: render a labelled Cancel action while waiting, and keep the arrow for Back.
| dialog's own profile step starts from the name given here. A relay that | ||
| already holds the key's profile, or a local default with a name, skips the | ||
| step. **Continue** is the dialog's profile step (`completeJoin`): the draft is | ||
| When the admitted relay profile has no trimmed nonempty name, the gate stays up |
There was a problem hiding this comment.
🤖 P3: docs drift from the implementation.
- This line says a relay profile without a nonempty name prompts for setup, but
decide()and the join path check!read.exists. Existing unnamed profiles skip the prompt, as the PR body intends. :5-7still describes a Community URL field on welcome, and says first-run copy never names the sign-in provider (the button now says Builderlab).:45-50still promises Cancel sign-in while pending; see the existing :854 thread.
Update these with the behavior decisions.
Allow continuation only for unavailable or not-offered backup, preserving the saved key and remaining setup steps. Keep transient failures retry-only and restore recovery-authority disclosure before create and replacement uploads. Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account. Re-review of 43e17c2e (delta from b9a559d6).
Both blockers are resolved. Unsupported backup (unavailable / not_offered) now continues to community setup without skipping linking or transient retries, and the recovery-authority disclosure is back on create and start-over. Details are in the two threads.
The P2/P3 comments from the previous review are unchanged by this commit and still open for the author's decision: profile step exit, app-wide disabled fill, --radius-card collision, intro inert, icon-only Cancel and docs drift.
Validation: source review of the delta plus its new unit and journey tests. I did not run tests locally. Hosted CI was still pending at this snapshot (7 pass, 33 pending, Windows skipped). This is not an approval.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 43e17c2eed
ℹ️ 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".
| document.documentElement.dataset.welcomeIntro === "pending" | ||
| ) { | ||
| delete document.documentElement.dataset.welcomeIntro; | ||
| void invoke("plugin:window|show", { label: "main" }).catch(() => {}); |
There was a problem hiding this comment.
For an existing macOS installation without the newly introduced buzz-mac-welcome-intro.v1 marker, the bootstrap script hides the window and sets data-welcome-intro="pending" on every launch. When the saved identity resolves, this bypass shows the window but never writes the marker, so upgraded users repeat the hidden-window startup gate indefinitely rather than bypassing it once; record the intro as consumed when resolving directly to the existing-identity or error surface.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl (automated reviewer, via Wes's account): confirmed at 8ad5dd33, low impact (P3). onboarding-intro-init.js:9-21 holds the window hidden whenever buzz-mac-welcome-intro.v1 is unset. The bypass effect at Welcome.tsx:241-249 shows it but never sets the key, and useWelcomeIntro writes it only when the intro actually runs. So every upgraded macOS user with an identity starts hidden on every launch until the identity resolves (or the 8 s fallback fires), not just once. Whether to mark it consumed on bypass is a product call. Doing so means a later reset would skip the intro.
| const decoded = decode(npub); | ||
| const label = | ||
| decoded.type === "npub" ? abbreviatedPublicKey(decoded.data) : npub; |
There was a problem hiding this comment.
When a returning user needs to compare the account-linked identity with another device before restoring or importing, this reduces the key to an abbreviated label and leaves the full npub only in a pointer-oriented title, with no Copy action. That removes the reliable keyboard, touch, and assistive-technology verification path required by docs/identity.md:122-128; render the full npub and restore its Copy control.
AGENTS.md reference: AGENTS.md:L90-L93
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 Carl (automated reviewer, via Wes's account): confirmed at 8ad5dd33 as a docs/code contract gap (P3). WelcomeBack.tsx:26-27,69-74 renders only abbreviatedPublicKey with the full npub in title, while docs/identity.md:122-124 still promises the full npub with Copy. Safety impact is limited because the import refuses any other key and then shows the full expected npub (KeyImport.tsx:86-91). The compact row looks like a deliberate design choice, so either restore a full-npub/Copy path or update the docs to match.
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f29a5e5a42
ℹ️ 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".
Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz> Signed-off-by: klopez4212 <[email protected]>
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account. Re-review of 8ad5dd33 (delta from 43e17c2e: picker-motion test repair and community origin on first-run join).
Verdict: no blockers in this delta. The first-run join now shows the canonical community URL before consent and before profile publication, which also covers the consent-origin concern deferred earlier. The test repair pauses picker animations when the DOM changes instead of on queued transitionrun, with the same assertions. That is test-only.
Two Codex P2s posted since the last review are confirmed as P3 and answered inline: the intro marker is never consumed on bypass, and Welcome back no longer offers a full npub or Copy despite the docs. Earlier P2/P3 comments remain for the author's decision.
Validation: source review of the delta and the new threads; no local runs. Hosted CI is green at this head (43 pass, Windows skipped). Live hosted sign-in/backup and packaged fresh-install testing are still unexercised. This is not an approval.
…xecution-support-for-native-buzz-33ea2bea * origin/main: (74 commits) feat(host): harden byte lifecycle and add protected image upload (#2199) Updated onboarding (#2183) perf(relay): read each signed roster once in large channels (#2244) AIDA-2346: Keep spoilers hidden in reminder previews (#2261) fix: AIDA-2371 Keep sidebar selection pill aligned during resize (#2182) fix(relay): skip background rereads of profiles a read found absent (#2147) fix: prevent startup logo selection (#1823) fix(agents): send sign-in evidence with agent Git credentials (#2240) perf(mentions): stop scanning every member on each mention keystroke (#2246) fix(attachments): send image dimensions so previews don't shift (#2262) feat(shortcuts): add Slack-style hotkeys (#2170) perf(plugins): share host request clients across calls (#2243) fix(identity): keep federated sessions and agent grants in one keychain item (#2225) fix(profiles): evict the least recently requested profile (#2255) fix(unread): repair unread from the recovery tick only every 15 minutes (#2252) AIDA-2374: Use readable labels in chat titles (#2185) perf(profiles): keep verified names checked through a planned renewal (#2234) feat(plugins): let a plugin ask Buzz to review an install (#2059) fix(identity): sign FI builds in at first run through the native session (FOUNDATION: App.tsx, services.ts) (#2249) Place profile verification beside the verified name (#2224) ... Signed-off-by: Daniel Rouleau <[email protected]>
* commit 'c8e882165': (50 commits) Scope execution targets by harness session Read durable execution state before admission feat(host): harden byte lifecycle and add protected image upload (#2199) Updated onboarding (#2183) perf(relay): read each signed roster once in large channels (#2244) AIDA-2346: Keep spoilers hidden in reminder previews (#2261) fix: AIDA-2371 Keep sidebar selection pill aligned during resize (#2182) fix(relay): skip background rereads of profiles a read found absent (#2147) fix: prevent startup logo selection (#1823) fix(agents): send sign-in evidence with agent Git credentials (#2240) perf(mentions): stop scanning every member on each mention keystroke (#2246) fix(attachments): send image dimensions so previews don't shift (#2262) feat(shortcuts): add Slack-style hotkeys (#2170) perf(plugins): share host request clients across calls (#2243) fix(identity): keep federated sessions and agent grants in one keychain item (#2225) fix(profiles): evict the least recently requested profile (#2255) fix(unread): repair unread from the recovery tick only every 15 minutes (#2252) AIDA-2374: Use readable labels in chat titles (#2185) perf(profiles): keep verified names checked through a planned renewal (#2234) feat(plugins): let a plugin ask Buzz to review an install (#2059) ... Signed-off-by: Daniel Rouleau <[email protected]> Co-authored-by: Codex <[email protected]>
…ection * origin/main: (39 commits) Authorize local apps to use Buzz MCP tools (block#2211) feat(bestie): choose the private channels Bestie can read (block#2036) Polish message actions, spacing, and membership notices (block#2258) test(unread): prevent startup catch-up in receipt reserve journey (block#2274) test(search): wait for rejected join clearance to settle (block#2278) test(reactions): provide stable emoji media for icon rendering (block#2279) feat(profiles): reserve profile room for an open channel's roster (block#2269) feat(host): harden byte lifecycle and add protected image upload (block#2199) Updated onboarding (block#2183) perf(relay): read each signed roster once in large channels (block#2244) AIDA-2346: Keep spoilers hidden in reminder previews (block#2261) fix: AIDA-2371 Keep sidebar selection pill aligned during resize (block#2182) fix(relay): skip background rereads of profiles a read found absent (block#2147) fix: prevent startup logo selection (block#1823) fix(agents): send sign-in evidence with agent Git credentials (block#2240) perf(mentions): stop scanning every member on each mention keystroke (block#2246) fix(attachments): send image dimensions so previews don't shift (block#2262) feat(shortcuts): add Slack-style hotkeys (block#2170) perf(plugins): share host request clients across calls (block#2243) fix(identity): keep federated sessions and agent grants in one keychain item (block#2225) ... # Conflicts: # src/features/messages/ThreadPanel.tsx
Streamlines onboarding with shared layouts, clearer recovery failures, and full-page community joining. Builderlab key creation, import and replacement now back up automatically; transient failures retain the key and offer retry, including reauthentication for expired sessions. Builds/accounts that explicitly do not support backup offer Continue without backup and retain the remaining setup steps. Create and replacement disclose that Block can restore the key, including its use elsewhere. Independent imports remain local. Restoration uses one status screen and continues directly after installing the matching key. Existing community profiles are preserved, including unnamed profiles. After admission, completion moves forward and local-save failures offer retry. Sign-in explicitly names Builderlab; setup copy explains automatic backup without an extra backup-choice screen. Validation: 466 identity tests pass after the blocker fixes, including unsupported-backup continuation and guards against bypassing linking/transient failures. TypeScript, lint and design checks passed during implementation. Chromium/WebKit fixture previews cover light/dark, narrow/wide layouts and custom community terms acceptance in onboarding and Add Community. Independent review blockers were fixed. Remaining validation: live hosted sign-in/backup, live communities with differing requirements, and packaged fresh-install testing. Lost-key guidance still returns through Go back rather than offering a direct Restore from backup action. Hosted checks must be assessed on the new head. <details> <summary>Earlier onboarding snapshots (before automatic-backup refinement; some screens are superseded)</summary> | Screen | Light | Dark | | --- | --- | --- | | Get started with Buzz |  |  | | Private key import |  |  | | Returning account |  |  | | Lost key |  |  | | Optional key backup |  |  | | Getting ready |  |  | | Restore key |  |  | | Saving restored key |  |  | | Recovery failure |  |  | | Recovery unavailable |  |  | | No backup found |  |  | | Backup does not match |  |  | | Partial recovery |  |  | | Connect to your community |  |  | | Community selection |  |  | | Required profile name |  |  | | Community access and consent |  |  | | Community profile confirmation |  |  | </details> ### CI browser test repair Capture model-picker transitions at the DOM-change boundary rather than queued transition events, which WebKit can deliver after the entrance finishes under load. No product code, browser cases, assertions, or timeout budgets changed. The complete three-case file passes in Chromium and WebKit (6 cases; 13.9s local macOS run). Hosted validation remains pending for this repair. --------- Signed-off-by: klopez4212 <[email protected]> Co-authored-by: Terp <5856f6900e8e0a4e0ea1f4f8d2ee33e4753d2119bc122f94ecafc660b46c0511@buzz.block.builderlab.xyz>
Since the onboarding update (#2183) the macOS main window is cleared at both the NSWindow and the WKWebView layer so the one-time welcome intro can show the desktop. On every other launch the only opaque pixels come from page CSS, which WebKit paints through its GPU process. When that process hangs at login, the page still shows the window before first paint and it stays fully see-through, with no sign that Buzz is open. Setup now gives the main NSWindow a dynamic background that resolves to --bg-app for the system appearance (#f1f1f2 light, #101010 dark), so a launch that never paints shows a solid blank window instead of nothing. The intro clears it again in onboarding_preserve_window, which already runs on the still-hidden window before the intro shows it, so the intro keeps its current look. isOpaque, the webview's drawsBackground and tauri.macos.conf.json are unchanged, and no command or capability is added. Buzz's own light/dark choice lives in page storage, so when it differs from the system appearance the window shows the other mode's --bg-app until the page paints. Signed-off-by: Matt Toohey <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]> Signed-off-by: Matt Toohey <[email protected]>
…3342) ## Problem On macOS, Buzz sometimes opens to a window that never paints. It has been seen when macOS reopens Buzz at login after a restart. The app is otherwise alive: the page loads and its JavaScript and IPC keep running. Only quitting and reopening Buzz clears it. The cause is in WebKit, not in Buzz: 1. On the first page connection, WebKit's GPU process makes a blocking audio-codec lookup. Right after login the macOS audio services it calls are still starting, so the lookup is slow. 2. WebKit gives the GPU process 3 s to answer. When it misses that, WebKit declares it unresponsive and asks launchd to kill it. 3. macOS refuses the kill with `Could not signal service com.apple.WebKit.GPU: 1: Operation not permitted`. WebKit stops tracking the process anyway and never starts a replacement. 4. The page stays connected to the abandoned process, so nothing is painted again. Since #2183 the main window is clear at the native level, so the failure shows as a fully see-through window with only the traffic lights. ## What this changes - **Solid native window outside the welcome intro.** The main `NSWindow` gets a background that matches `--bg-app`, so a launch that never paints shows a blank window instead of nothing. The intro clears it again and keeps its current look. - **Kill the GPU process WebKit abandoned.** A new macOS-only module, `src-tauri/src/gpu_reaper.rs`, checks once a second which GPU process the main webview reports. A process that was reported earlier, is no longer reported for two checks, and is still the same running WebKit GPU binary gets `SIGKILL`. WebKit then starts a new GPU process and the page paints without a reload. - **Skip the check while the webview has no content process.** WebKit also reports no GPU process in that state, while other webviews keep using the healthy one. This came out of review of the previous commit. - **Write the watcher's messages to the unified log.** An app opened from Finder, the Dock or at login has no stderr, so the watcher's own lines were lost and a kill that macOS refused left nothing to find. The four messages now also go to the unified log at notice level. The watcher never kills a process it did not first see reported by Buzz's own webview. It does nothing if the private selectors are missing. ## Research Upstream WebKit: - [WebKit bug 324797](https://bugs.webkit.org/show_bug.cgi?id=324797): "GPU process termination fails with EPERM after unresponsiveness, leaving WKWebView blank". Open and unassigned. The log lines match ours. The reporter induced the stall on purpose and has not established a natural trigger. - [WebKit bug 331017](https://bugs.webkit.org/show_bug.cgi?id=331017) and its unmerged fix [WebKit/WebKit#80415](WebKit/WebKit#80415): the GPU process is killed as unresponsive while it is only slow to start on a loaded machine. That fix moves the start of the checks to the end of GPU process initialisation. Our stall happens later, on the first page connection, so it may not cover this case. - [WebKit commit 5e174d0](WebKit/WebKit@5e174d0) for bug 300013: adds the Opus and Vorbis capability check that runs when a GPU connection is created. This is the lookup our GPU process was blocked in. Other apps with the same failure: - [Saqoosha/Canopy#299](Saqoosha/Canopy#299), merged: a WKWebView pane stayed blank after the GPU process hung at launch inside CoreAudio. The app now finds the abandoned process through `_gpuProcessIdentifier` and kills it, after which "WebKit launch[es] a new GPU process, and the page paints at once". This PR takes the same approach. - [DigitalN/mac-engine-room#5](DigitalN/mac-engine-room#5), merged: "After a reboot the dashboard opened at login and stayed blank." The GPU process hung setting up audio, the kill failed with the same message, and the app now ends that process itself. - [bambulab/BambuStudio#12575](bambulab/BambuStudio#12575), open: on macOS 27.0.1 every web view is blank because the GPU process hangs during CoreAudio initialisation. It does not mention the failed kill. - [yetone/magpie#3272](yetone/magpie#3272), open: the same three log lines on macOS 27.0.1, with the GPU process still alive afterwards. It happened mid-session, not at launch. We found no report from Tauri, wry or tao, and no public report that names the Opus lookup as the trigger. ## Verification Done, on macOS 27.2: - Eight unit tests for the watcher pass, including one that kills a child process the test starts. Removing either safety check makes tests fail. - `clippy --all-targets -D warnings` is clean for the crate. - In a standalone WKWebView test app, WebKit's own kill was refused every time. Killing the abandoned GPU process from the app brought up a new one and both web views painted in under half a second. Reloading the page did not recover it. - Hardened-runtime, ad-hoc signed release builds of Buzz were launched with their own GPU process stopped. In five runs before the logging commit, the watcher killed it 1.1 to 2.0 s after WebKit gave up, WebKit started a new GPU process within half a second, and the page painted without a reload. Two of those runs were launched hidden through LaunchServices and never activated. - A build with the logging commit was run once the same way, launched in the background through LaunchServices. The watcher's `Killed WebKit GPU process` line was in the unified log at default level 1.77 s after WebKit gave up. launchd's kill record followed and a new GPU process was running. - The timer keeps its rate while the window is hidden. In a hidden, never-activated test app, App Nap began 31 s after launch and a 1 s timer still fired every 0.6 to 1.4 s for the next 270 s. The stall happens within the first 5 s. - The solid background was checked on screen in light and dark mode, and the welcome intro still shows the desktop. - An independent review found no way for the watcher to kill another app's process or a reused pid. Its one warning is fixed by the third commit. Not done: - The kill has not run from a Developer ID signed, notarized build. Signed builds come only from `main`, so the restart test below runs on the first signed build after merge. - The watcher's `Could not kill` line has not been produced by Buzz, because no kill was refused in any run. The same log call with a refused-kill text was read back from a standalone hardened-runtime binary. - Timer behaviour on battery power and with the screen locked has not been measured. A missed process only leaves the window blank, as it is today. Known limits, left for a follow-up: - The native colour follows the macOS appearance. A user whose Buzz theme differs can see the other colour until the page paints. - The session that shows the welcome intro keeps a see-through native window until the next launch. ## Test plan - [x] Normal launch in light and dark mode: solid `--bg-app` before first paint, usual look afterwards. - [x] Fresh welcome intro: the desktop still shows through. - [x] Release build, window visible and hidden: `kill -STOP` the build's own `com.apple.WebKit.GPU` process and wait. The log shows `GPUProcessProxy::didBecomeUnresponsive`, then the watcher's kill, then a new GPU process, and the page paints again. - [ ] Signed build: restart with Buzz open and confirm it reopens to a painted window. Then check the log: - `log show --last 30m --predicate 'process == "Buzz" AND eventMessage CONTAINS "GPU process"'` - `log show --last 30m --predicate 'process == "launchd" AND eventMessage CONTAINS "exited due to SIGKILL | sent by Buzz"'` 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Matt Toohey <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
Streamlines onboarding with shared layouts, clearer recovery failures, and full-page community joining. Builderlab key creation, import and replacement now back up automatically; transient failures retain the key and offer retry, including reauthentication for expired sessions. Builds/accounts that explicitly do not support backup offer Continue without backup and retain the remaining setup steps. Create and replacement disclose that Block can restore the key, including its use elsewhere. Independent imports remain local.
Restoration uses one status screen and continues directly after installing the matching key. Existing community profiles are preserved, including unnamed profiles. After admission, completion moves forward and local-save failures offer retry. Sign-in explicitly names Builderlab; setup copy explains automatic backup without an extra backup-choice screen.
Validation: 466 identity tests pass after the blocker fixes, including unsupported-backup continuation and guards against bypassing linking/transient failures. TypeScript, lint and design checks passed during implementation. Chromium/WebKit fixture previews cover light/dark, narrow/wide layouts and custom community terms acceptance in onboarding and Add Community. Independent review blockers were fixed.
Remaining validation: live hosted sign-in/backup, live communities with differing requirements, and packaged fresh-install testing. Lost-key guidance still returns through Go back rather than offering a direct Restore from backup action. Hosted checks must be assessed on the new head.
Earlier onboarding snapshots (before automatic-backup refinement; some screens are superseded)
| Community profile confirmation |
| 
|
CI browser test repair
Capture model-picker transitions at the DOM-change boundary rather than queued transition events, which WebKit can deliver after the entrance finishes under load. No product code, browser cases, assertions, or timeout budgets changed. The complete three-case file passes in Chromium and WebKit (6 cases; 13.9s local macOS run). Hosted validation remains pending for this repair.