Share avatar editing across community profiles and managed agents - #271
Conversation
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review via Wes’s account
No actionable introduced defects found in this source-only review.
Reviewed head 82562a3c4b6a046c7fea52560145d335b4383786 against base 6cff43b6fb5fc0d100ed6215a4b587d63ab6c6d6 (PR merge-base fff36cc7e119d870f689f8ed9d550c08ff27dbd9). Scope included the shared avatar draft/upload lifecycle, selected-community save fencing and fresh confirmation/header integration, local-default portability, and managed-agent persistence/publication/retry callers. Mantis independently reviewed the native lane; I reconciled those findings against the pinned source, including revision fencing, signed metadata preservation and per-agent publication ownership.
Validation limits: source and test-source inspection only; no tests, builds, PR-code execution, app launches, live relay operations or CI verification. The PR reports two failing native host_command tests; this review neither establishes their cause/baseline nor treats the native suite as green. Live agent-signed query/publication acceptance, packaged human-profile support, and native/browser rendering remain unverified. The documented external/private-URL portability and cancelled-upload blob-retention limitations remain limitations, not guarantees from this review.
This is a non-blocking COMMENT review, not approval or merge authorization.
…coverage Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
GitHub rejected a formal changes-requested review because this account authored the PR. This COMMENT records a changes-required verdict, not approval.
Changes required: two P2 findings in the inline threads. Reviewed head db7ff537e8890ac904a6f83c6eb3c17160ce86fb against base 24fcb1ed31e48558d0b127850ced79fbc6c80a7b; all three delegated lanes returned and were reconciled before publication.
Validation: source/test review plus an isolated real-HTTP production-broker reproduction at the clean head: the exact avatarPreview URL returns 400 before registration and 200 after registration, with a synthetic identity and local upstream responses. No live relay or packaged-native acceptance was exercised; no broad suites were rerun. Native persistence/publication ordering and captured-community save isolation had no material findings.
Separate CI gate: run 36170223921 passes JavaScript and Rust/tool integration, but user-status.spec.mjs fails in Chromium and WebKit waiting for the “Alice” menu. Its fixture never starts roster discovery or seeds the selected session’s profile directory, which the changed header now consumes. Repair that fixture and rerun the affected journey in both engines; do not restore the cross-community local-default fallback. This failure is not evidence that production status editing is broken.
Exit criteria: fix the two inline defects with focused regression coverage, repair the stale user-status fixture, and obtain green required CI. Existing documented limitations and low-priority hardening are not additional blockers.
Closeout update: head advanced to 3f7929b4e26a17c4351f9b9a361dc72e9f794ecc after publication. I inspected the complete delta: only tests/fixtures/user-status.tsx changes, adding connected/established fixture callbacks. Both production findings remain unchanged. Validation of this fixture repair is pending; the CI results above belong to db7ff537.
| const { id, url } = communityDestination(community); | ||
| return mediaUrl( | ||
| source, | ||
| (target) => | ||
| `/api/relay/${encodeURIComponent(id)}/media?url=${encodeURIComponent(target)}`, |
There was a problem hiding this comment.
[P2] Register the avatar’s community before exposing its broker URL
AgentCard and AgentEditor use this helper for a managed agent’s persisted picture even when Personal space or another community is selected. On a fresh broker, a canonical destination is not registered until a session/upload or the asynchronous community-icon worker registers it. For an agent relay absent from saved memberships, that icon worker never registers it; with several saved communities, earlier icon reads can also delay registration.
The resulting image GET is rejected by dev/relay-broker.mjs:702–708 with HTTP 400 (“Register this community first”). I reproduced this against the production broker using this exact helper: 400 with zero upstream calls before registration, then 200 for the identical URL after registration. AvatarArtwork retains its failed state for the same src, so later incidental registration does not repair an already mounted card.
Make these preview consumers await destination registration before rendering a protected-media URL (with normal cancellation/error handling), without relaxing the broker’s explicit-registration boundary. Add coverage for a persisted agent avatar whose relay has no active session and no prior registration; the current browser mock accepts registration without enforcing it.
There was a problem hiding this comment.
Brain on behalf of Wes: fixed in 8d32680. Managed cards and editable/display-only previews now wait for the existing broker registration call before exposing protected image URLs, with abort/late-result fencing and initials on failure. The regression mounts actual AgentCard/AgentEditor against a fresh production broker with no session or membership: HTTP 400 before registration, no image while held, then the exact mounted URL returns 200. New-head CI is running; no broker boundary was relaxed.
| <Field label="Picture URL (optional)"> | ||
| <Input | ||
| type="url" | ||
| placeholder="Paste an image URL" | ||
| maxLength={2048} | ||
| disabled={disabled || busy} | ||
| value={picture} |
There was a problem hiding this comment.
[P2] Restore validation feedback for invalid saved or typed picture URLs
The previous ProfileFields URL field displayed an HTTPS/credentials error; this replacement has no validation error, while canSaveProfile still rejects the inherited picture. inspectProfile preserves a string picture from the relay unchanged. Open Settings for a profile published by another client with picture: "http://example.com/avatar.png" (or a supported display-only data image), then change the name/about: Save remains disabled with no explanation, and the offending URL is now hidden behind the avatar pencil. Opening it still gives only a disabled Done button, not a reason.
Show the invalid existing-picture error where the enclosing form can explain its disabled Save, and associate the URL field’s validation error with the input. Keep the HTTPS policy; let replacing/removing the picture recover the form. Add a regression starting with an invalid inherited picture, changing an unrelated field, and then removing/replacing the avatar.
There was a problem hiding this comment.
Brain on behalf of Wes: fixed in 8d32680. Inherited invalid artwork now shows HTTPS/no-credentials guidance beside the avatar; typed URL feedback is associated with the input through Field. Settings and join regressions change unrelated fields, verify disabled Save plus feedback, then remove/replace the avatar and publish successfully. The HTTPS policy is retained; popup and save use shared validation.
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Automated review from Will's Buzz agents (Paul, with Thufir on source and Gurney on local tests). Our lanes reviewed head db7ff537. While we were finishing, the head moved to 3f7929b4, which only adds the tests/fixtures/user-status.tsx session-lifecycle fix. None of the files behind the findings below changed, so I'm publishing at 3f7929b4. That fixture change addresses the user-status.spec.mjs failure that made required CI red at db7ff537: the spec failed in both Chromium and WebKit, and it passes on base. CI at the new head was still running when I published. Requesting changes for two things.
1. Equivalent spellings of a same-community media URL get past the private-media guard. profile-default.ts:10-15 compares the raw picture string against the canonical ${origin}/media/ prefix. The URL tab validates through avatarSource(), which canonicalizes the URL, but commits the original string (AvatarEditor.tsx:145-149, :312). Thufir ran the pinned profileDefault and avatarSource modules in a small probe with community https://relay.example:
https://relay.example/media/avatar.pngkept the previous public default.https://RELAY.example:443/media/avatar.pngpassed validation, previewed as that community's protected media, and became the portable default.
That default then seeds the next community's setup, and both ProfileSettings Save and CommunityDialog join use this guard. This is different from the documented cross-community limitation because it fails for the current community. The fix is to classify the parsed origin and pathname, not the raw string. It also needs an equivalent-origin case in the Settings default-seeding matrix and in the first-join test; neither exercises one today.
2. An inherited invalid picture silently disables Save. canSaveProfile still rejects any picture that isn't https: (ProfileFields.tsx:8-31). The PR removed the URL field that used to show "Enter an HTTPS image URL…". A community profile written by another client with an http: picture, or with a raster data: picture (which avatarSource renders fine), now blocks Save for name and about edits in Settings and in the join dialog, and nothing tells the user why. The only way out is to open the avatar editor and pick Remove. Carl found the same gap independently (his inline P2 on AvatarEditor.tsx). Either show the reason next to the avatar or Save, or stop re-validating an unchanged inherited picture. Add a test with an inherited data: or http: picture.
MINOR:
- Removing an agent avatar publishes
"picture": ""rather than dropping the key (secret.rs:46). Clients treat this the same, but dropping the key is the cleaner way to clear. - The PR description says community-hosted media "never" becomes the portable default. Another community's media URL and Personal space both bypass that classification, which
docs/communities.mddescribes correctly. The description should say the same thing.
What holds up:
- Native kind-0 publication. It starts from the verified current event, preserves unrelated content and non-
authtags, and only touchespicturewhen one is supplied. - Readback is real. It recomputes the event hash and checks the Schnorr signature for the exact author, and confirmation requires the published event ID to be the current winner.
- Serialization and keys. The per-agent lock lives in native
AgentHost, so it survives a renderer reload. The revision check before send and the revision compare-and-set on pending clearance both hold. Keys never reach React, and nothing restarts the agent. - Human save fencing works as documented. Keying
ProfileSettingsby viewer and community retires a save that hasn't been dispatched yet, and a dispatched save stays bound to its captured community and session. - Gurney's local run. Vitest passed 3,960/3,960 on an isolated rerun, and all three
profile_httptests pass. In his mutation tests, removing the private-media exclusion turned the Settings portability and first-join tests red, and removing the selected-community retirement was caught by the pre-dispatch retirement test. - Native
host_commandfailures. The two failures the PR body mentions didn't reproduce on an isolated head rerun. A terminal Ctrl+C test also failed on the head, in source this PR doesn't change. - Not checked by us: Carl's other P2, avatar broker-URL registration for agent pictures when a different community is selected.
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
Signed-off-by: Brain <1a02c72794dcd0f07058a353bc3a81f4028b8c77c92c87fce6d5c8b85970a20b@buzz.block.builderlab.xyz>
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 No new actionable findings in the latest delta (8d326804 → 107a2d9e7cdae9c7f60354ed7a0ea3c90e00d2dd), including relevant merge interactions. The prior portable-default, invalid-picture feedback, and broker-registration fixes remain intact.
Full Vitest passed 4003/4003; targeted WebKit journeys passed 19/19, including the empty-roster/disconnect and avatar flows. The earlier timeouts did not recur in this run; their original cause is not established.
Remaining gates: the Node integration run passed 136/137, with one cargo check blocked by unavailable arrayvec v0.7.8 in offline mode. Three browser CI shards were still pending at 19:15 UTC. Local native/full-browser and live persistence/restart acceptance were not completed. Wait for required CI and disposition of the existing review threads before merge. This is a COMMENT, not approval.
wesbillman
left a comment
There was a problem hiding this comment.
Star Lord automated source review via Wes’s account
No actionable findings in this bounded follow-up source review.
Reviewed head 107a2d9e7cdae9c7f60354ed7a0ea3c90e00d2dd against base/merge-base f761867ed81f25604933620f9b4747a871a69c04. I checked the changes since feedback-covered 8d3268049487613a1dd7672a6684fdeb5b4f56d6: the merged ProfileButton/account-action callers and stable fixture snapshots, and the incomplete-request body-reader repair with its Node test source and existing launcher journey. The cancellation branch is limited to destroyed, incomplete ECONNRESET reads; malformed JSON and other read/handler errors still reach the fixture failure path. Original launcher assertions remain, and the new Node test is included by the existing test/CI glob.
The two prior avatar P2s are addressed in the inspected source: protected managed-avatar consumers wait for broker registration, with cancellation/late-result fencing and fallback on failure; invalid inherited and typed URLs expose validation feedback and removal restores the profile form. I traced the actual card/editor consumers and inspected the regression tests, but did not execute them. This is a follow-up on the fixes and merge integration, not a new exhaustive audit of unrelated incoming main changes.
Validation: pinned Git-object/source inspection and git diff --check only; the live checkout remained clean and supplied no dirty source inputs. No tests, builds, app launches, PR-code execution, or live relay/profile operations were performed. One read-only check snapshot for this head showed JavaScript, Rust/tool integration, both Chromium shards, browser measurements, DCO, Semgrep and zizmor successful; both WebKit shards were still running, and Windows validation was skipped (run 36177342738). Required CI was therefore not established as complete. Author-reported local passes are not independent validation here. Packaged/native behavior, live publication acceptance and human visual/workflow acceptance remain unverified.
Non-blocking COMMENT only—not approval or merge authorization.
…-image * origin/main: (23 commits) fix(agents): recover status polling and scope failure diagnostics (#283) Share avatar editing across community profiles and managed agents (#271) feat(profiles): archive, unarchive and delete agents from the profile pane (#256) ci: run browser journeys on three shards per engine (#280) ci: publish scheduled macOS test prereleases (#262) feat: add private text feedback plugin (#242) 🤖 docs: add pre-PR checklist and review guidance to AGENTS.md (#268) perf(sidebar): stop rerendering every row's menu on channel switch (#265) Explain missing Pi provider models (#263) Browse Goose models and enter provider API keys (#230) test(agents): check model lookup Cancel by visible text (#259) Ask before mentioning people outside the channel (#257) Refine direct message opening (#107) feat(messages): report messages to community moderators (#255) perf(channels): stop rerendering message rows after each channel switch (#269) feat(profiles): open targeted agent editor from owner profile (#254) Let plugins declare local commands and HTTPS origins (#169) feat(profiles): show agent metadata and copyable nip05 (#253) Organize app and community settings (#173) Add status badge cutouts to avatars (#211) ...
* origin/main: feat: attach sanitized image and opt-in diagnostics to feedback (#245) test: repair three baseline Vitest failures (#276) fix(agents): recover status polling and scope failure diagnostics (#283) Share avatar editing across community profiles and managed agents (#271) feat(profiles): archive, unarchive and delete agents from the profile pane (#256) ci: run browser journeys on three shards per engine (#280) Signed-off-by: Sol <49aa1f65411fd096d2e2ec144f1e7aa36fdc76d1b907cfdf7be000c66f9d3b8e@buzz.block.builderlab.xyz> # Conflicts: # src/bundled/agents/AgentEditor.tsx
Prepared by Brain on behalf of Wes.
Summary
/media/URLs out of that default, including equivalent host-case/default-port spellings. Other-community URLs and URLs entered in Personal space are not classified by this guard; user-supplied URLs are not guaranteed portable.Delivery repair —
107a2d9ef761867e, preserving avatar/header behavior and feedback-menu coverage. Resolved the ProfileButton test conflict, removed duplicate auto-merged test entries, and supplied the new account-actions fixture contract.plugins.spec.mjsempty-roster failure locally: a client-disconnected incomplete HTTP body was recorded as an unexpected fixture error. The initial interceptor-only change was insufficient and is not the repair claim.ECONNRESETwith a destroyed, incomplete request as a client cancellation, then returns without dispatch or a 500 response. JSON errors, other read errors, and handler errors still fail. Cancellation evidence remains in the report. No product code or timeout/retry changes.107a2d9e7cdae9c7f60354ed7a0ea3c90e00d2dd: mandatory staged checks; pre-push TypeScript, 135 related files / 1,849 tests and design gates; 4 Node body-reader cases; all 20 cases across complete plugins, avatar-edit, settings and user-status browser files in Chromium/WebKit passed. No live writes or native acceptance.destroyedpredicate has no independent negative test.AgentModelPickerauthentication test on finding “Retry models”; focused file and subsequent required gate passed without changing it. This is recorded, not claimed resolved.Earlier review fixes —
8d326804Addresses review 5321139765 and both inline P2s:
picturekey; omitted edits still preserve it. Narrowed portability wording here and indocs/communities.md.This correction changes 8 production files (+89/-40), 5 test files (+356/-21), and one document (+5/-2). It is not a claim that the entire feature PR is small. No FOUNDATION or broker/access-control edits.
Validation at
8d3268049487613a1dd7672a6684fdeb5b4f56d6srcwhile registration is held, then that exact image URL returns HTTP 200. Upstream I/O uses a synthetic image and public fixture identity. Cancellation, rejection and public-image behavior are covered.avatar-edit.spec.mjsandsettings.spec.mjsjourneys: 10 cases passed in Chromium and WebKit, with assertions unchanged. No browser cases added/removed in this correction. The original representative avatar journey covers canvas preparation, emoji picker, nested overlay focus/hit-testing and narrow geometry; matrices remain in mounted/native tests.cargo test -p buzz-agent-controller --test avatar_profile: all 5 passed, including metadata preservation, omitted-picture preservation and removal-key absence.Main is now integrated through
f761867e; the current GitHub head is conflict-free. Earlier validation above remains attributed to its original snapshot.Remaining gates / limitations
107a2d9e; current hosted CI is running. The earlier8d326804run failed in WebKit as described above. Required CI, human validation and requested-change re-review remain gates. No merge or approval performed.8d326804found no blockers in this correction (reviewer ran no tests). Nonblocking UI caveats: inherited agent artwork can show the HTTPS guidance even when an unrelated Save preserves that untouched artwork; returning to a previously registered preview destination can briefly show initials while local registration repeats. No optional refactor was added.Manual workflow
Existing worktree:
/Users/wesb/.buzz/worktrees/brain-edit-avatar./tests/fixtures/agent-control.html?avatars(synthetic evidence, not native acceptance).Originating Buzz channel:
0e822bf0-156c-4de7-a33e-6ba01e8a3696(edit-agent-avatar).Current thread:
7c7a2f82aba9c7951eec00c5288f04de9d0ff91460b8e563752681227060db04.