fix(desktop): keep managed agent avatars usable across communities - #7732
Conversation
Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz>
🔐 Codex Security Review
|
jedwards27
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Reviewed: 779af8886caae1317b4de962082429867ab61503..925781eece66d5f6a4b084b3b3abd6309cda357e (exact head 925781eece66d5f6a4b084b3b3abd6309cda357e)
Risk: critical — this crosses community tenancy, managed-agent identity, authenticated media, renderer↔Tauri IPC, persistence/retry, and profile publication boundaries.
As :bot: Jude’s code review agent, I found no unresolved author-actionable defect. The implementation advances the repository contract that identity is portable while profiles and media remain per-community (VISION.md:50-56): it localizes only configured-community media into the caller-pinned destination before signing the replacement kind:0, while preserving the saved source and existing retry behavior.
Behavior/contracts traced
- Renderer permission-list normalization, serialized IPC updates, startup’s stable-latest wait, and visible reload recovery (
desktop/src/features/communities/communityRelaySet.ts;useCommunityInit.ts:141-161,338-376). - Fixed agent signer and pinned destination through query/localization/publication; failure before publication leaves the prior profile intact (
desktop/src-tauri/src/commands/agents_profile.rs:201-327;relay.rs:549-605). - Origin validation and authenticated-request containment: HTTPS (or loopback HTTP), no credentials/path/query/fragment, configured-source membership rechecked after the target probe, no redirects, 30-second deadlines, bounded bodies (
profile_avatar.rs:22-53,75-152,261-283). - Byte/hash/MIME preservation and destination descriptor binding: downloaded bytes are SHA-256 checked and uploaded unchanged; returned origin/hash/MIME/size must match (
profile_avatar.rs:153-209). This preserves already-sanitized animated bytes rather than re-encoding them. - Removed/unreachable source recovery: no authenticated read after removal; an already-published signed same-hash destination projection can be reused, otherwise the unchanged source remains retryable (
profile_avatar.rs:91-103,212-259). - Opt-out is checked before localization and again before publication (
agents_profile.rs:209-214,284-315). Concurrent same-hash copies are content-addressed/idempotent.
Findings: no blocking or non-blocking code defect found.
Author action: none.
Verification owner: release/dogfood owns the remaining live/native observation; the still-running repository CI jobs own their normal merge gate.
Validation at matching clean HEAD
- Independent product/adversarial lane:
cd desktop && pnpm test→ 6,508/6,508 passed;pnpm typecheckpassed;just desktop-tauri-testpassed; Tauri fmt and Clippy passed in default andmesh-llmgraphs;git diff --checkpassed. - Independent systems/integration lane:
just _ensure-sidecar-stubs && cd desktop/src-tauri && cargo test relay::profile_avatar -- --nocapture→ 10 passed, 0 failed. - Production-seam coverage exercises two-community copy/edit/restart, fixed signer/destination, source loss and removal races, upload/profile rejection then retry, redirects, hash/descriptor/size/type rejection, existing projection reuse, persistence/reconciliation, and opt-out (
profile_avatar/tests.rs:193-552). Frontend tests cover trust normalization and restoration ordering; the author reports mutation removal of serialization/stable waiting fails those regressions, satisfyingTESTING.md:25-29. - CI observed green for macOS and Windows builds, Rust lint/Windows, relay-backed Desktop integration shards, three completed smoke shards, Semgrep, zizmor, and DCO. At submission,
Desktop Coreand smoke shard 4 were still running with no failed check; those remain external merge gates rather than author defects. - Immediately before submission: live PR head = local HEAD =
925781eece66d5f6a4b084b3b3abd6309cda357e; tree clean. Authenticated reviewerjedwards27differs from authorwesbillman.
Manual/native evidence: no packaged live two-community avatar journey was run. No native screenshot/video is claimed.
Residual risk: a real authenticated hosted-relay/WebView run has not independently witnessed animated rendering across restart, nor the user-visible old-avatar→retry recovery during a deliberate live outage. The production-seam tests establish byte identity, fail-before-publish behavior, and successful later reconciliation; release/dogfood should exercise those two native journeys. No author action is required.
* fix(ci): don't run desktop tests for purely mobile client changes (block#7709) Mobile-only PRs currently trigger desktop CI because the desktop test filter's glob inadvertently matches every path outside Tauri. Fix that filter so that pure mobile changes skip desktop builds, tests, and their relay artifact producer. ### Verification - 12 regression tests exercise the exact pinned paths-filter action on mobile, desktop, Tauri, backend, mixed, workflow, and documentation changes. - Restoring the bad glob makes the mobile-only regression test fail. - The filter tests, existing required-context isolation contract, workflow syntax validation, and Biome pass locally. --------- Signed-off-by: Tom Brow <tomb@block.xyz> (cherry picked from commit 01b6174) (cherry picked from commit 89e4ce2e1e7f9f17a1d752c05fd0a166ca0b8be1) * fix(desktop): keep managed agent avatars usable across communities (block#7732) ## Summary Fix Desktop-managed agents losing avatars after joining another community. The saved persona/instance stores one desired source URL, but authenticated media belongs to a community. Republishing community A’s URL in B does not make that image accessible in B. - Before comparing or publishing kind-0, reuse or copy configured-community avatar media into the pinned destination. Keep exact image bytes, including animation, and leave the saved source unchanged. - Keep the previous profile on transfer/publication failure so existing reconciliation can retry. Reuse destination bytes during source outages; after source removal, reuse only an already-published signed destination picture with the same original content hash. - Refresh source permissions through narrow, serialized IPC without reconnecting or resetting the active community. Startup waits until the latest permission update completes before restoring agents. ### Related issue Refs block#2366 (avatar portion only, not runtime availability). Related block#2659 deliberately excludes avatar/media mirroring; this change addresses that media boundary without introducing runtime fan-out. Searched existing issues/PRs for `avatar community` before opening. ## Contract and safety Desktop-managed profiles still follow the saved persona source; this does not introduce community-specific persona editing. Agent-managed profiles still opt out of automatic reconciliation. Authenticated transfer requests use the fixed agent key/NIP-OA tag, origin-scoped Blossom auth, and the pinned destination, never mutable active-workspace owner credentials. They refuse redirects, enforce 30-second request timeouts and bounded bodies, verify SHA-256 and detected image MIME, and validate the returned descriptor’s origin/hash/MIME/size. Source permission is rechecked after an awaited destination HEAD miss. The removed-source fallback verifies the destination profile’s signature, kind, signer, origin, and original-media hash without contacting the removed source. Ordinary external images retain passthrough behavior and receive no community credentials. No public-media policy change, image-reader URL rewriting, new persistence, background service, or global profile merge. ## Testing Normal commit and push hooks passed on exact head `925781eece66d5f6a4b084b3b3abd6309cda357e`; working tree clean before and after: - Desktop formatting/lint, typecheck, and **6,508 tests**. - Full native workspace suite, including **3,204 desktop-library tests** (19 ignored), integration tests, terminal crates, and doc tests. - Native Clippy on default and `mesh-llm` workspace/all-target configurations with warnings denied; repository file-size and branch-skew gates. Regression coverage exercises production reconciliation/shared publication against two local HTTP fixtures: add/edit/restart, upload and rejected-publication retry, removed-source local-image preservation, fixed signer/destination, redirect/hash/descriptor/size failures, revocation during pending HEAD, and opt-out no-I/O. Mounted initialization tests cover inactive-list changes without workspace reset, two deferred permission updates, and rejection of either update. Earlier working-tree validation at base `4ab4f786085a23fe6126529861840eff6048ceee` also passed the E2E frontend build and four Chromium mock-bridge community flows (boot, switch, active removal/fallback, leave-final/setup). Subsequent production changes only serialize permission writes and wait for a stable latest update; the complete suites above cover the final commit. Mutations independently removing serialization or stable waiting failed the new regression. ## Remaining validation and limitations - No live production avatar was changed. Native GUI rendering against two hosted communities remains to be verified with a build containing this fix; local HTTP fixtures and the browser mock bridge do not establish that outcome. - A removed/unreachable source with no existing destination copy cannot be recovered automatically. Choose a reachable image/source in that case. - This is a publication/reconciliation fix, not a visual redesign. No before/after native GUI screenshots are claimed. Broad repository CI remains required; it was not duplicated locally. ## Review Independent security/scope and caller/ordering reviews are clear after the bounded fixes. Final hook-ordering reread covered the staged tree that became this commit. Commit authorship/sign-off use the implementing Carl identity and configured Carl signing key. Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> (cherry picked from commit 8953cbf) (cherry picked from commit f0a773ed029a9f0f751a1231642a442b26137590) * feat(db): expose connection setup metrics (block#7286) ## Why Current pool metrics show checkout outcomes and pool state after the fact. They do not show when a checkout began or which writer-connection setup step failed. During startup and pool growth, operators need to distinguish pool saturation from a slow or unsafe connection setup. ## What - Count every instrumented database checkout when it starts, with a fixed operation label. - Measure writer-pool creation and every physical writer connection across `physical_connect`, `created_at_floor`, `session_timeouts`, `isolation`, and `ready`. - Publish fixed-cardinality start, terminal-outcome, duration, and waiter metrics without database URLs, SQL, raw errors, connection ordinals, or per-connection lifecycle logs. - Document how to identify the active setup bottleneck during a rollout or incident. ## How The existing typed checkout wrapper records starts and current waiters. The huddle-history path uses that wrapper instead of a raw pool checkout. The production SQLx `after_connect` hook records start, terminal outcome, and duration for each sequential safety step. A drop guard records cancellation once if setup exits before a terminal result. Fixed enums keep every label bounded. For a phase, `started_total - sum(attempts_total)` is the number of in-progress attempts on that pod. Because setup is sequential, a later phase starting also proves the earlier phases succeeded. The database path emits metrics only. This PR intentionally does not add per-connection lifecycle receipts or direct `stderr` writes. ## Risk Medium. This changes the production writer-pool `after_connect` hook and adds one counter update to instrumented checkout paths. It does not change the database safety statements or their failure behavior. The telemetry has fixed labels and no per-connection identifiers. ## Testing At exact head `ffb5fcb114a9c986ffd0cb2cfc9d413a32d5b1aa`, a release relay was started against isolated local PostgreSQL, Redis, and MinIO services. The main health endpoint returned `ok`, readiness returned `{"status":"ready"}`, the connection-step metrics were exported, and no database lifecycle receipts were emitted. A release CLI then created a channel, sent a message, and read the same event back successfully. The earlier staging deployment used pre-rebase head `f58e9480a4f068db0c591f604fd6800fdd4bfc45`. The deployed multi-architecture image came from [GitHub Actions run 33780888255](https://github.com/block/buzz/actions/runs/33780888255), manifest `sha256:b9351fa644e08376cbe1999f9bee311d33d1799a68eadef7929c4f862a832fec`. The [staging deployment](https://github.com/squareup/builderbot-platform-core-infrastructure/pull/317) brought both pods in ReplicaSet `buzz-6c8758bd7d` to Ready with zero restarts, and the connection metrics produced data in the [rollout dashboard](https://app.datadoghq.com/dashboard/tm7-qxr-wt2/buzz-startup--rollout-safety). That staging image predates the cleanup that removed per-connection lifecycle logs; the metric schema and database safety statements are unchanged. ## Verification - `cargo test -p buzz-db -- --test-threads=1`: 128 tests passed across the package and integration target; 255 opt-in tests remained ignored. - Four opt-in production-path PostgreSQL regressions passed: initial minimum connections, post-startup pool growth, isolation failure, and session-timeout setup failure. - All 35 relay media tests passed against the isolated PostgreSQL database. - Rust formatting, workspace clippy, desktop/Tauri clippy, web checks, mobile analysis, security checks, and file-size policy checks passed. - The repository-wide unit stage also exposed three unrelated existing `buzz-acp` failures: one timing-sensitive keepalive test and two environment-default tests. ## Bigger picture This is the database-metrics part of the startup and rollout observability work. The early-startup lifecycle foundation merged in block#7258, so the rebase removed that duplicate commit from this PR. Originating discussion: buzz://message?channel=6ac85131-70cd-4bda-a031-38d34114934e&id=fa2bed181c092697210a60bb6eedc55a665d5c1c6cabc1413a04686647011f71 Generated with Codex --------- Signed-off-by: Ravneet Arora <rarora@squareup.com> (cherry picked from commit 4c2086c) (cherry picked from commit b42caabf790ac13f4507e97936243959934ae461) * test(db): freeze migration 46 against the upstream 0046 collision Upstream block/buzz shipped 0046_storage_acco...[truncated] (cherry picked from commit 6bc8126296473332be78cb31a9f95c313165d6ac) --------- Signed-off-by: Tom Brow <tomb@block.xyz> Signed-off-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Signed-off-by: Ravneet Arora <rarora@squareup.com> Co-authored-by: Tom Brow <tomb@block.xyz> Co-authored-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <32a2e2c9d428ee08902cab75d956da2c1d235a22d4766b0dd4138bf6e2e5db1d@buzz.block.builderlab.xyz> Co-authored-by: ravarora2 <130506156+ravarora2@users.noreply.github.com>
Summary
Fix Desktop-managed agents losing avatars after joining another community. The saved persona/instance stores one desired source URL, but authenticated media belongs to a community. Republishing community A’s URL in B does not make that image accessible in B.
Related issue
Refs #2366 (avatar portion only, not runtime availability). Related #2659 deliberately excludes avatar/media mirroring; this change addresses that media boundary without introducing runtime fan-out. Searched existing issues/PRs for
avatar communitybefore opening.Contract and safety
Desktop-managed profiles still follow the saved persona source; this does not introduce community-specific persona editing. Agent-managed profiles still opt out of automatic reconciliation.
Authenticated transfer requests use the fixed agent key/NIP-OA tag, origin-scoped Blossom auth, and the pinned destination, never mutable active-workspace owner credentials. They refuse redirects, enforce 30-second request timeouts and bounded bodies, verify SHA-256 and detected image MIME, and validate the returned descriptor’s origin/hash/MIME/size. Source permission is rechecked after an awaited destination HEAD miss. The removed-source fallback verifies the destination profile’s signature, kind, signer, origin, and original-media hash without contacting the removed source.
Ordinary external images retain passthrough behavior and receive no community credentials. No public-media policy change, image-reader URL rewriting, new persistence, background service, or global profile merge.
Testing
Normal commit and push hooks passed on exact head
925781eece66d5f6a4b084b3b3abd6309cda357e; working tree clean before and after:mesh-llmworkspace/all-target configurations with warnings denied; repository file-size and branch-skew gates.Regression coverage exercises production reconciliation/shared publication against two local HTTP fixtures: add/edit/restart, upload and rejected-publication retry, removed-source local-image preservation, fixed signer/destination, redirect/hash/descriptor/size failures, revocation during pending HEAD, and opt-out no-I/O. Mounted initialization tests cover inactive-list changes without workspace reset, two deferred permission updates, and rejection of either update.
Earlier working-tree validation at base
4ab4f786085a23fe6126529861840eff6048ceeealso passed the E2E frontend build and four Chromium mock-bridge community flows (boot, switch, active removal/fallback, leave-final/setup). Subsequent production changes only serialize permission writes and wait for a stable latest update; the complete suites above cover the final commit. Mutations independently removing serialization or stable waiting failed the new regression.Remaining validation and limitations
Review
Independent security/scope and caller/ordering reviews are clear after the bounded fixes. Final hook-ordering reread covered the staged tree that became this commit. Commit authorship/sign-off use the implementing Carl identity and configured Carl signing key.