Repository navigation
feat(identity): first-run Builderlab account client — sign-in without a plugin session, kgoose reads, link/unlink, backup, saved communities — and its dev seed - #1818
Conversation
c133edb to
eccd287
Compare
b202f48 to
6e41931
Compare
eccd287 to
1ee6dc5
Compare
… a plugin session, kgoose reads, link/unlink, backup, saved communities — and its dev seed
All of first run's network logic, no UI. Nothing on main calls it yet; the
Welcome flow (PR 07) is the caller.
`src/bundled/builderlab/oauth/first-run.ts` (new): binds a `Host` to the
Builderlab plugin's own bundled native grant (`block.builderlab`/`bundled`,
granted at build time from `BUZZ_BUILDERLAB_URL` and the tenant table) and
exposes `firstRunCredential(signal, origin?)`, the plugin's
`browserCredential` run before any plugin session exists. That credential
is separate from the Settings session and is never persisted.
`src/features/identity/builderlab-account.ts` (new): `FirstRunAccount`, the
kgoose reads and writes first run needs, every request a POST with `{}` or
a body, `X-BB-Session-Credential` from the first-run session and `Origin`
set to the login origin that signed the user in, through `bundledHost.request`:
- `linkedKey` (`/v1/buzz/nostr-identities/current`): a 2xx with `identity`
null or absent is *not linked* (kgoose never sends `MISSING_MAPPING` on
this route); a present identity whose `pubkey_hex` is not 64 lowercase
hex fails — a linked account must never be given a second key.
- `ownedCommunities` (`/v1/buzz/communities/list`, archived and insecure
hosts excluded) and `knownCommunities`
(`/v1/buzz/known-communities/list`, read with the Builderlab plugin's own
`listedAddress`, exported from `known-communities/client.ts` for this; a
row this client cannot read rejects the whole list).
- `bind(pubkey)`: reads `current` first and returns when the account already
holds this key, else challenge → native `identity_sign_builderlab_binding`
→ verify, which must confirm this key. 409 `identity_already_bound` /
`pubkey_already_bound` take the hosted-communities card's copy.
- `unbind(old, new)` (`/nostr-identities/delete`): the route takes no key
and deletes whatever is bound, so it sends the delete only while `current`
still answers the old key, counts as done when the delete echoes the old
key or a re-read shows none or the new key, and fails without Try again
if the old key is still bound. The real fix is an `expected_pubkey_hex`
on the delete, noted in a comment.
- `backUp(pubkey)`: refuses with `BackupError("unavailable")` unless
`backupOfferedFor(backup, origin)` holds — this build's uploader backs up
to Builderlab and the sign-in is at the build's own portal (a tenant's
credential is never handed to the native uploader) — then `backUpWith`.
- `backupStatus()` (`/v1/buzz/nsec-backups/status`): the listed hex keys;
"unavailable" without a request unless offered, and on a 404; other
answers map to the backup failure kinds.
- `recovery`: the native recovery driver, so the dev seed can replace it.
- `saveCommunity(url)` (`/v1/buzz/known-communities/add`,
`wss://host[:port]`).
- Shared `call`/`post`: transient failures (no status, 429, 5xx) are
retryable, 4xx refusals and rejected shapes are not; any HTTP 401 is
"Your Builderlab sign-in is no longer valid."; failure copy names the
step and status, never a response body; kgoose codes the card's table
lacks get first-run copy (`invalid_origin`, `invalid_challenge`,
`challenge_*`, `too_many_attempts`; `challenge_expired/consumed/not_found`
retryable since Try again fetches a fresh challenge); the correlation ID
stays off the page and goes to a development-only `console.warn`.
Dev seed (`first-run-seed.ts`, new; `seededAccount`):
`BUZZ_DEV_FIRST_RUN_ACCOUNT` — `unlinked`, or `{"npub","communities",
"known","backups","recovery","backup":"<kind>"|"fail-once","unlink":<status>}`
— replaces the kgoose reads, the binding, the unlink, the backup and backup
recovery, and nothing else, on a dev server. `vite.config.ts` validates it
(explicit `.ts` imports so Node's native config loader stays warning-free)
and defines `VITE_BUZZ_FIRST_RUN_ACCOUNT` for `serve` only; builds define it
empty. Bad values are rejected without being echoed. A developer's real
account binding never changes under a seed.
Docs: `docs/configuration.md` (the seed row and "First-run account seed
(development)" section), `.env.example`, the Builderlab README's first-run
paragraph.
Tests: `builderlab-account.test.ts` (1,337: every route's exact request at
a tenant and at the build URL; linked/not-linked readings; the bind
sequences incl. the already-bound no-op and 409 from verify; unbind's
current-then-delete with echo and re-read outcomes; backUp's tenant guard
and handoff; backupStatus mappings; saveCommunity; the retryability, 401
and correlation-ID rules; the seeded account), `first-run-seed.test.ts`
(268: every field, rejections without echo, the seeded backup/unlink/
recovery behaviours), `oauth/first-run.test.ts` (the bundled grant request
shape with and without an origin), and the seed lines of
`browser-host/vite-config.test.mjs` (serve vs build define, rejections).
Stack: PR 05 of the onboarding stack, on 04; imports 02 (`destination`),
03 (`backup`, `backup-failure`, `service`) and 04 (`oauth/browser`). 06–08
build on it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
6e41931 to
1d5b1b1
Compare
1ee6dc5 to
1d6ceda
Compare
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.
Three P2 issues are detailed inline: challenge-origin binding, first-run transport after disabling Builderlab, and concurrent unlink safety. Optional: clarify that the seed/Welcome documentation describes the later stack integration.
Rocket, Mantis and Groot completed independent lanes. Reviewed head 1d5b1b1e8f8278405bda3f883ea0483a7fb94526, base 1d6cedae14db488ec7ef86ccf16325823128226c. This is account-client groundwork; the UI caller arrives later. Source-only: no tests, app execution, live account operations or CI verification; cancellation/focus behavior in the eventual UI remains unverified.
| if (!reply) throw failure(step, response); | ||
| const { challenge_id, nonce, verification_code, origin, expires_at } = | ||
| reply as Record<string, unknown>; | ||
| return { challenge_id, nonce, verification_code, origin, expires_at }; |
There was a problem hiding this comment.
🤖 P2 — Bind the returned challenge to the selected portal before signing. The request supplies portal, but the response's origin is forwarded unchecked to the native signer. BuilderlabChallenge::template accepts any build-granted origin, so a response from one tenant can obtain a binding signature scoped to another granted portal; the subsequent verify response check happens after the signature has already been disclosed. Reject origin !== portal before invoking the signer, and cover a reply naming a different granted origin with a test asserting no signature or verify request.
| runCommand: () => Promise.resolve(null), | ||
| request: (request) => | ||
| invoke("plugin_host_request", { | ||
| id: manifest.id, |
There was a problem hiding this comment.
🤖 P2 — Handle a persisted disabled Builderlab plugin before first-run sign-in. This transport calls plugin_host_request, whose host_grants lookup requires the bundled plugin to be enabled (crates/plugin-manager/src/lib.rs:1112–1122). Disabling Builderlab is persisted, and ordinary sign-out retains that registry (src-tauri/src/sign_out.rs:665–669). Thus this first-run helper can finish browser authentication and then fail credential exchange locally on the next setup. Give the app-owned flow narrowly scoped access independent of Settings-plugin enablement, or explicitly handle the disabled state before opening login, retaining configured-origin enforcement. Cover disabled-plugin → plain sign-out → first-run login.
| if (before !== old) throw new Error(OTHER_KEY); | ||
| const removed = await call( | ||
| "account unlink", | ||
| "/v1/buzz/nostr-identities/delete", |
There was a problem hiding this comment.
🤖 P2 — Make unlink conditional on the old binding before exposing Start over. As the comment above acknowledges, the preflight read and unconditional delete are separate operations. If another device replaces the old binding between them, this delete removes that replacement; the later read sees no key and returns success. Readback cannot recover the deleted binding, so the replacement/third-key protection in this API's contract is not enforced. Require a server-checked expected old key under the binding lock and send it here; until that contract is available, keep this destructive path unavailable. Add a regression for a binding change between the read and delete.
Onboarding stack (merge in order, top to bottom):
Depends on: PR 02 (
destination), PR 03 (backup,backup-failure,service; this layer addsisBackupFailureKindtobackup-failure.tsfor the seed guard, whichvite.config.tsloads through Node's native loader), PR 04 (oauth/browser). PRs 06–08 build on it.Diff: production +835/−2; tests +1697/−0; docs +56/−0
What this layer is
All first-run network logic, no UI.
oauth/first-run.tsbinds aHostto the plugin's bundled grant and exposesfirstRunCredential(signal, origin?);builderlab-account.tsisFirstRunAccount:linkedKey(200-with-null ⇒ not linked),ownedCommunities,knownCommunities(vialistedAddress),bind(current-first, challenge → native signer → verify),unbind(delete only while the old key is bound, verified by echo or re-read),backUp(tenant guard, thenbackUpWith),backupStatus,recovery,saveCommunity,backupOfferedFor; the sharedcall/posthelpers with retryability (no status / 5xx / 429), kgoose code tables, 401 mapping, correlation-ID handling;seededAccount+first-run-seed.ts(BUZZ_DEV_FIRST_RUN_ACCOUNTparser, Node-loadable with.tsimports) wired throughvite.config.tsforserveonly.Review focus
Originand the first-run session credential and goes through the bundled grant — never the Settings session's client. The credential is never persisted.linkedKey: a 2xx withidentitynull/absent is not linked; a present identity with a badpubkey_hexfails closed (a linked account must never be given a second key).unbind: kgoose's delete takes no key, so the client readscurrentfirst and verifies by echo or re-read; a third key or a still-bound old key fails rather than resolving quietly. The real fix (expected_pubkey_hexon the delete) is a kgoose change, noted in a comment.backUp's tenant guard: a tenant's credential never reaches the native uploader, which cannot tell where a credential came from.Checks run on this branch's tree
--no-verify; the hosted checks on this PR are the verification. The plan's checks for this layer were:vitest run src/features/identity src/bundled/builderlab browser-host/vite-config.test.mjs,tsc --noEmit,biome check.Native evidence
None on this tree. The native first-run recordings and the hands-on checks on the branch were made on the full tree, which only the DO-NOT-MERGE PR 09 reproduces (it adds the isolated debug Keychain slot). Treat every layer below 09 as unit/integration-tested only.
🤖 Generated with Claude Code