Conversation
Signed-off-by: npub1z3hmzc9ryehxzedl5wzlvpyvja0d483peaja5zt6pd0209f9x2jspe2dxh <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: npub13n66s06epmqf2kc3v373ez8hj65cuzyvxzjf93vwpervxqn2u7jq2qd9je <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
Signed-off-by: npub13n66s06epmqf2kc3v373ez8hj65cuzyvxzjf93vwpervxqn2u7jq2qd9je <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
Signed-off-by: npub1z3hmzc9ryehxzedl5wzlvpyvja0d483peaja5zt6pd0209f9x2jspe2dxh <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: npub1z3hmzc9ryehxzedl5wzlvpyvja0d483peaja5zt6pd0209f9x2jspe2dxh <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: npub13n66s06epmqf2kc3v373ez8hj65cuzyvxzjf93vwpervxqn2u7jq2qd9je <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
Signed-off-by: npub13n66s06epmqf2kc3v373ez8hj65cuzyvxzjf93vwpervxqn2u7jq2qd9je <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
Signed-off-by: npub1z3hmzc9ryehxzedl5wzlvpyvja0d483peaja5zt6pd0209f9x2jspe2dxh <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
8652610 to
aaf4be8
Compare
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ebed7a843
ℹ️ 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".
| //! SQLite storage for the single-node community profile. | ||
| //! | ||
| //! This intentionally covers only community bootstrap, channels, memberships, | ||
| //! and relay ownership. Event and user persistence lands with PR 2. |
There was a problem hiding this comment.
Keep local communities gated until SQLite supports relay traffic
When the localCommunities preview is enabled, the desktop starts the normal relay router with this intentionally partial backend and a dummy PostgreSQL pool. Every NIP-42 login calls moderation_restriction_state, which still uses that pool and fails closed, and event REQ/write methods such as query_events_routed and insert_event do the same. Consequently the experiment cannot authenticate or store/load messages; implement the required SQLite dispatches before exposing the option, or keep the local relay unavailable until those slices land.
Useful? React with 👍 / 👎.
| unsafe { | ||
| libc::kill(self.child.id() as i32, libc::SIGTERM); | ||
| } |
There was a problem hiding this comment.
Replace the unsafe SIGTERM call
On every Unix shutdown or community switch, this path enters an explicit unsafe block solely to signal the child relay. The repository prohibits unsafe code, so use a safe process/signal wrapper while preserving the graceful SIGTERM behavior.
AGENTS.md reference: AGENTS.md:L115-L115
Useful? React with 👍 / 👎.
| Err(error) => return Err(error), | ||
| } | ||
| } | ||
| Err(last_error.expect("at least one local relay startup attempt")) |
There was a problem hiding this comment.
Remove production expect calls from the relay supervisor
This new production supervisor uses expect() here and in both stderr-tail mutex accesses, while the single-node startup path also expects relay-key parsing to succeed. Repository policy requires production paths to propagate proper errors rather than panic, so replace these invariant and poison assertions with fallible handling.
AGENTS.md reference: AGENTS.md:L116-L116
Useful? React with 👍 / 👎.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Reviewed base 8f29ecbdb06ba042b1cc4d833d3d4d74b115fbf7 through exact head 7ebed7a843bce5e3cf447ef703e2733a63e9a04b.
Required change
The first-run local-community flow hides sidecar/apply failures and leaves onboarding stuck.
useCommunityInit catches applyCommunity failures and records them only in its local result (desktop/src/features/communities/useCommunityInit.ts:227-259). While onboarding has an active transaction, App deliberately does not render CommunityApplyErrorScreen (desktop/src/app/App.tsx:520-549). The connecting transaction advances only when targetIsReady becomes true (desktop/src/app/App.tsx:466-496), which cannot happen after that caught failure. Finally, the onboarding curtain displays retry/error UI only for transaction.error, but this failure is never copied there (desktop/src/features/onboarding/ui/CommunityOnboardingFlow.tsx:482-507).
Consequently, a missing/corrupt sidecar, exhausted bind retries, or corrupt service key leaves the user indefinitely on “Connecting securely…” with only Cancel, rather than exposing the failure and a retry path. This is particularly important because those are explicit failure modes of the new local-relay lifecycle.
Author action: propagate the apply failure into the active onboarding transaction or otherwise render the apply-error recovery UI above the transaction curtain. Add a regression covering a local-community onboarding apply failure and proving that the error plus a functional retry/change-community path is shown instead of an endless connecting state.
Verification owner: author for the regression and fix; reviewer to re-run the focused onboarding failure scenario and review the new head.
Validation and confidence
- Independently traced the React state transition from onboarding
connectingthroughuseCommunityInit,targetIsReady, and both competing error surfaces at the pinned head. - Live GitHub head remained
7ebed7a843bce5e3cf447ef703e2733a63e9a04b; required CI checks reported success for that SHA. - Confidence gap, not additional author action: no exact packaged desktop artifact was exercised in this review. The code-path defect is deterministic and does not depend on native observation; the release/canary owner should still validate bundled-sidecar startup and recovery on produced macOS/Windows/Linux artifacts.
- The independent systems lane did not return a final report before the review deadline, so no claims from that unfinished lane are included. The product/UI finding above was independently re-traced before submission.
jedwards27
left a comment
There was a problem hiding this comment.
:bot: Jude’s code review agent
Team verdict: REQUEST CHANGES. This supersedes my narrower review submitted moments earlier and integrates the completed systems and product/UI lanes for base 8f29ecbdb06ba042b1cc4d833d3d4d74b115fbf7 through exact head 7ebed7a843bce5e3cf447ef703e2733a63e9a04b.
Required changes
1. The enabled local relay cannot authenticate clients or store/query normal events
Db::new_sqlite installs an SQLite backend plus a deliberately unusable lazy Postgres pool (crates/buzz-db/src/lib.rs:650-675), but the normal relay paths are not dispatched to SQLite. In particular:
moderation_restriction_stateunconditionally queries the Postgres pool (crates/buzz-db/src/lib.rs:4445-4452), while NIP-42 treats that lookup error as fail-closed authentication denial (crates/buzz-relay/src/handlers/auth.rs:119-165).- Core
insert_event,query_events, and routed query methods likewise unconditionally use Postgres (crates/buzz-db/src/lib.rs:1690-1800).
The UI exposes local-community creation and the sidecar becomes healthy, but an actual client cannot complete auth or persist/read ordinary messages on that advertised SQLite path.
Author action: implement the SQLite dispatches needed for the supported local-relay workflow and prove auth plus event publish/query in an end-to-end single-node test, or keep local-community creation unavailable until that slice exists.
Verification owner: author for the targeted relay integration test; reviewer for replacement-head tracing and rerun.
2. First-run apply failure is hidden behind an endless connecting curtain
useCommunityInit records applyCommunity failure only in its local result (desktop/src/features/communities/useCommunityInit.ts:227-259). With an onboarding transaction present, App suppresses CommunityApplyErrorScreen (desktop/src/app/App.tsx:520-549), and the transaction advances only when targetIsReady becomes true (desktop/src/app/App.tsx:466-496). The curtain only exposes transaction.error, which is never populated from this failure (desktop/src/features/onboarding/ui/CommunityOnboardingFlow.tsx:482-507). Missing/corrupt sidecar, exhausted bind retry, or corrupt service key therefore strands the user on “Connecting securely…”.
Author action: propagate the apply failure into onboarding or render the apply-error recovery UI above the curtain. Add a regression forcing apply_workspace rejection and proving visible error plus a functional retry/cancel or change-community path.
Verification owner: author + Desktop E2E/CI; reviewer re-runs the focused failure workflow.
3. New production code violates the repository's explicit safety/error-handling rules
The root contributor guide says “No unsafe code” and forbids new production unwrap()/expect() calls (AGENTS.md:114-117). This change adds unsafe { libc::kill(...) } in desktop/src-tauri/src/local_relay.rs:79-85, production expect() calls at local_relay.rs:143,313,325, and another at crates/buzz-relay/src/main.rs:1251-1252.
Author action: replace these with safe, fallible handling consistent with repository policy.
Verification owner: author + affected Rust package CI; reviewer checks the delta.
4. Cross-platform UI copy claims the feature is Mac-only
desktop/src/features/communities/ui/CommunityCreationChoice.tsx:5-6 says “Private to this Mac,” but this PR wires/bundles the sidecar for Windows and Linux as well. The adjacent label already uses “this device.”
Author action: use platform-neutral “this device” copy or platform-specific copy, and update the relevant frontend assertion/snapshot.
Verification owner: author + frontend CI; product/UI reviewer checks the rendered text.
Validation and confidence gaps
just desktop-cipassed in a clean pinned worktree: frontend check, 4,579 tests, and TypeScript typecheck; post-runHEADwas the reviewed SHA and the tree was clean. Biome warnings were informational and outside this PR's files.- Source tracing confirmed application/health/metrics listeners are loopback-constrained and sidecar wiring is present in Tauri/release configuration.
- Live GitHub required checks report success at the reviewed SHA.
- Confidence gaps, not additional defects: local
cargo test -p buzz-dband Desktop/Tauri Rust validation each exceeded a 600-second dependency compilation/startup window; no packaged native artifact was exercised. Rust CI and release owners should verify those lanes on the replacement head/artifacts.
🤖
Summary
local_communitiesexperiment gate.add_membermatch the public Postgres contract transactionally, including key validation, private-channel admission, role/last-owner restrictions, and original-inviter preservation across active re-add and soft-delete reactivation.brother-darryl/local-mode-pr0-seams) and retargets tomainafter that PR merges.Fix-round follow-up
if/elsehandling and moving the local auth-tag backfill toagent_auth.rs, avoiding anagents.rsfile-size-ratchet exception.buzz-relaysidecar across supported platforms.Integration debt
Conflicts with current
maininWelcomeSetup.tsxandonboarding.spec.tsfrom #5310 onboarding polish. Deliberately deferred: resolve in the rebase-onto-mainat retarget time after #5506 merges, with delta review of the resolutions.Related issue
N/A — stacked follow-up to #5506.
Testing
cargo test -p buzz-db --lib: 97 passed, 154 ignored.