[thebrandonlucas]

Problem

buzz-app moved its keys from the dev.local.buzz.foundation.identity keychain entry into one combined entry: service buzz-desktop, account secrets, a JSON map with the human key under identity. The GPUI app reads only the old entry, so a buzz-app user must paste their nsec on first start (#86).

Change

flowchart TD
    start["Launch"] --> load{"dev.local.buzz.foundation.identity"}
    load -- "found" --> done["Signed in"]
    load -- "empty" --> marker{"identity-adopted marker?"}
    marker -- "present" --> screen["Import or create screen"]
    marker -- "absent" --> read["Read buzz-desktop / secrets,<br/>map entry identity"]
    read -- "nsec" --> save["Save to this app's entry,<br/>write marker"] --> done
    read -- "no identity" --> mark["Write marker"] --> screen
    read -- "error (denied, busy)" --> screen

Loading

• buzz-identity: adopt_buzz_app() reads buzz-app's entry, takes the identity nsec, and saves it under this app's entry. It wipes the other map values (agent keys) and never writes buzz-app's entry, so buzz-app keeps working with the same key.
• community::identity::restore: copies the key once. The marker .buzz-gpui--buzz-desktop.identity-adopted is beside the sign-out markers, outside app data, so signing out or erasing does not bring buzz-app's key back. A read error is logged and retried at the next launch.
• README: describes where the app reads the identity.

The format follows upstream desktop/src-tauri/src/secret_store.rs and app_state.rs in block/buzz.

Validation

• New unit test: the human key is taken from the map; a map without identity, bad JSON, and a bad nsec are handled.
• cargo clippy -p buzz-identity -p buzz-gpui --features voltage-sandbox --all-targets -D warnings: passes.
• On macOS, with this app's entry deleted and buzz-app's entry present: the app asked for keychain access, copied the identity, wrote the marker, and signed in with the same public key.

Limits

• Stacked on build(gpui): build the app flake on macOS #88 (the macOS flake), so the change can be run with nix run on macOS. Merge build(gpui): build the app flake on macOS #88 first.
• Tested on macOS only. On Linux and Windows, buzz-app uses the same service and account through the keyring crate, and buzz_keychain reads the same entry, but I did not run it there.
• No test for the marker logic in community.rs; it calls the OS keychain.

Fixes #86 · Linear DEV-2978

Summary by CodeRabbit

• New Features
  • Buzz can restore an identity from buzz-app’s saved keyring entry when no identity is already saved in Buzz. The original entry remains unchanged.
  • Existing Buzz identities and configured development identity files continue to be used as before.
• Bug Fixes
  • Signing out records that the identity was removed, preventing the saved buzz-app identity from being adopted again afterward.
• Documentation
  • Ubuntu installation guidance explains identity restoration and notes that macOS may ask for permission to access the keyring entry.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: voltagecloud/buzz
• Branch: fix/gpui-read-buzz-app-identity
• File: .coderabbit.yaml
• Commit: cde632b
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Essentials
• Run ID: 3e8ccfaf-b081-494a-baa6-5ca3fdf49b18

Commits

Reviewing files that changed from the base of the PR and between b1f7aad and cde632b.

Files selected for processing: 1

• apps/buzz-gpui/crates/app/src/community.rs

Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

Walkthrough

When GPUI has no saved identity and no development identity file is configured, it can copy the identity from buzz-app’s combined keyring entry into its own store. A marker controls repeated adoption attempts. The README describes this behavior and possible macOS access prompts.

Changes: Identity adoption

Layer / File(s)	Summary
Read and save the legacy identity apps/buzz-gpui/crates/identity/src/lib.rs	adopt_buzz_app reads and parses the identity from buzz-app’s secrets map, then saves it to GPUI’s store. It returns no identity if the source entry or identity key is absent, and reports malformed input. The test covers identity selection and malformed data.
Restore fallback and marker handling apps/buzz-gpui/crates/app/src/community.rs, apps/buzz-gpui/README.md	identity::restore attempts adoption when GPUI has no saved identity and no development identity file is configured. A marker controls repeat attempts. Keychain removal writes the marker before deleting the identity. Tests cover adoption retries and sign-out behavior. The README describes the lookup and possible macOS access prompts.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant Restore as identity::restore
  participant GPUIStore as GPUI identity store
  participant adopt_buzz_app
  participant BuzzAppStore as buzz-desktop secrets entry
  Restore->>GPUIStore: Load saved identity
  GPUIStore-->>Restore: No saved identity
  Restore->>adopt_buzz_app: Attempt adoption if marker is absent
  adopt_buzz_app->>BuzzAppStore: Read secrets map
  BuzzAppStore-->>adopt_buzz_app: Return identity key
  adopt_buzz_app->>GPUIStore: Save adopted identity

Loading

Suggested labels: human review wanted

Suggested reviewers: tee8z

Merge Risk: Minimal · up to cde63

The identity fallback and sign-out safeguards appear ready to merge, subject to normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: GPUI now reads the identity stored by buzz-app.
Linked Issues check		Issue [#86] requires fallback adoption when this app has no saved identity. identity::restore calls adopt_buzz_app() only after load() returns no identity. adopt_buzz_app() reads `buzz-desktop…
Out of Scope Changes check		The whole-PR changes are limited to the [#86] key adoption path, marker and sign-out integration, related unit tests, and README documentation. The marker and error handling support the requested migr…
Docstring Coverage		Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentati…

Full details: Linked Issues check

Explanation

Issue [#86] requires fallback adoption when this app has no saved identity. identity::restore calls adopt_buzz_app() only after load() returns no identity. adopt_buzz_app() reads buzz-desktop / secrets, extracts the JSON identity nsec, and writes only this app's entry. It does not modify buzz-app's entry. The code handles absent, malformed, and invalid data. The marker prevents repeat adoption and sign-out records it before key deletion. Unit tests cover adoption, marker behavior, storage errors, malformed data, and sign-out. The README documents the fallback. Linux and Windows checks are validation tasks, not coding requirements.

Full details: Out of Scope Changes check

Explanation

The whole-PR changes are limited to the [#86] key adoption path, marker and sign-out integration, related unit tests, and README documentation. The marker and error handling support the requested migration behavior. No unrelated product behavior appears in the reviewed changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 2 files.

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[tee8z]

Not approving yet: one blocking issue, plus two optional notes.

Blocking: sign-out brings buzz-app's key back for anyone who already had a key in this app

The adopted marker is written only when restore() reaches the load() == None branch. Users who already have a key in dev.local.buzz.foundation.identity never reach that branch, so they never get the marker. That includes everyone who pasted their nsec because of #86 and everyone whose key is older than buzz-app's move. For them:

1. They choose Sign out (or Sign out and erase). At the next launch, sign_out::boot() calls identity::remove(), which calls buzz_identity::forget() and deletes this app's entry. buzz-app's buzz-desktop/secrets entry is untouched.
2. restore() runs, load() returns None, and there is no marker, so adopt_once() calls adopt_buzz_app() and copies buzz-app's identity into this app's entry.
3. The app opens signed in with buzz-app's key. If they signed out to switch identities, they are now on buzz-app's identity without being told.

That breaks the contract stated in the PR and in the adopted_marker doc: "signing out or erasing does not bring buzz-app's key back".

Fix: record the marker as part of sign-out, before the key is deleted. In the keychain branch of identity::remove() (community.rs), write adopted_marker() first. If that write fails, return Err, so the sign-out marker stays and the next launch retries (Review-Proven Rule 1). Touching the marker whenever load() returns Some would also cover most users. It misses a sign-out that was already pending when they upgrade, because boot() deletes the key before restore() runs.

Optional

• A malformed entry is retried forever. Bad JSON in buzz-app's map, or a bad identity nsec, returns Err(Malformed), so no marker is written. The app then reads the entry again at every launch, and on macOS that means a keychain prompt each time. Malformed data won't fix itself, so write the marker in that case and keep retrying only for store errors (denied, busy).
• The marker logic has no test (Rule 3). If adopt_once took the adopt step as a parameter (impl FnOnce() -> Result<Option<Identity>, IdentityError>), a temp-dir test could cover three cases: marker present means no adopt, an error means no marker, and the sign-out path writes the marker.

The buzz_app_identity parsing matches upstream secret_store.rs (a HashMap<String, String> under secrets). The raw blob comes back Zeroizing, the other values are wiped, and the Occupied → load() race handling looks right.

Minimalism 9 · elegance 9 · correctness 7 (the sign-out issue above).

[tee8z]

Approving. `cde632be` resolves the blocker and both optional notes.

• Sign-out contract holds for every user. `remove_keychain_key` writes the adoption marker before `forget()`, and a failed marker write fails the removal. I followed that into `sign_out::boot`: `boot_with(…, identity::remove)` turns the `Err` into `exit_with`, so the sign-out marker stays pending and the next launch retries (Rule 1). `forget()` failing after the marker is written is also fine: the marker only prevents adoption, which is the contract after a sign-out anyway.
• Erase does not clear the marker. `Paths::targets` removes `data_dir()/buzz-gpui`, the local-data and cache folders. The marker is at the `data_dir()` root beside the sign-out marker, outside all three.
• Malformed is terminal, store errors retry. `Denied`/`Busy` leave no marker; `Malformed` writes one, so a bad buzz-app entry no longer prompts every launch on macOS.
• Tests bind the production seam (Rule 3). `adopt_once` and `remove_keychain_key` are the functions `restore()`/`remove()` call, with the keychain step injected; the five cases cover marker-present, store-error, malformed, sign-out ordering, and marker-write failure keeping the key.
• `adopted_marker() == None` (no data dir) disables both adoption and the sign-out guard together, matching `sign_out::boot`'s own "no data folder" behaviour.

Not verified here: a live macOS run of sign out → relaunch with buzz-app's entry present. The PR's own macOS check covers first-launch adoption.

Minimalism 9 · elegance 9 · correctness 9.