Skip to content

Refine agent catalog sharing#2439

Open
klopez4212 wants to merge 27 commits into
mainfrom
kennylopez-agent-catalog-sharing
Open

Refine agent catalog sharing#2439
klopez4212 wants to merge 27 commits into
mainfrom
kennylopez-agent-catalog-sharing

Conversation

@klopez4212

@klopez4212 klopez4212 commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add custom-agent catalog sharing and hide built-ins from discovery
  • let owners publish later catalog updates from Share or while saving edits
  • preserve agent type, model, and runtime across snapshot import/export
  • simplify agent and team entry points and tighten catalog layout

Screenshots

Agent actions

Agent actions

Team avatar stack

Team avatar stack

Catalog sharing

Catalog sharing

Publish while editing

Publish while editing

Publish from Share

Publish from Share

Catalog details

Catalog details

Test plan

  • just ci
  • focused agent catalog Playwright coverage

klopez4212 added a commit that referenced this pull request Jul 23, 2026
@klopez4212
klopez4212 marked this pull request as ready for review July 23, 2026 00:02
@klopez4212
klopez4212 requested a review from a team as a code owner July 23, 2026 00:02
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
klopez4212 added a commit that referenced this pull request Jul 23, 2026
@klopez4212
klopez4212 requested a review from wesbillman July 23, 2026 21:35

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking finding

[P0] “Share to catalog” never publishes anything to the community.

The new catalog state is entirely device-local: setPersonaCatalogShareLevel only writes persona IDs, memory levels, and a timestamp to window.localStorage, while catalogPersonas is still derived from listPersonas() (the local personas.json projection). The existing persona sync is explicitly filtered to the active user's own pubkey (authors: [pubkey] in usePersonaSync.ts), and this PR adds no relay event, query, or subscription for community catalog entries. Likewise, publishPersonaCatalogUpdates only replaces a local timestamp; it does not publish the definition or selected memory snapshot.

Consequently, after Alice chooses “Agent only”, “Agent + core memory”, or “Agent + all memories”, Bob sees nothing. Even Alice loses the catalog selection on another browser/profile/device because these three keys are localStorage-only. This contradicts the UI promise, “Let anyone in this community find and use a copy of this agent,” and makes the primary feature nonfunctional outside its mocked single-page E2E setup.

Please persist catalog publication as community-scoped relay data and populate discovery from those relay entries (including an explicit privacy/security contract for the selected memory level), rather than treating localStorage as publication state.

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent pass found one additional community-boundary issue beyond the existing publication blocker: the new visibility, memory-level, and published-version state is stored under process-global localStorage keys rather than being scoped to the active community (or identity). Sharing an agent while connected to community A therefore also marks that local agent as shared when the user switches to community B, and unsharing/publishing in either community mutates the other community's state. The eventual relay-backed catalog state needs community/relay + owner scoping end-to-end, including local pending/publication metadata.

I also traced deletion/unsharing, save-and-publish sequencing, catalog derivation, and the snapshot schema/import changes. I found no other independent blocker at this head. All applicable CI is green; per review policy I did not duplicate it locally.

Comment thread desktop/src/features/agents/lib/personaCatalogVisibility.ts Outdated
@klopez4212

Copy link
Copy Markdown
Contributor Author

🤖 Addressed this P0 in e17d283.

  • Replaced localStorage catalog state with owner-authored, parameterized-replaceable kind 30178 relay events. Discovery now queries all community authors and subscribes live.
  • Publishing uploads a sanitized JSON snapshot and records its hash, size, and relay reference; unsharing publishes an explicit replacement head.
  • Added strict client and relay allowlists: catalog data is plaintext, secrets, environment values, auth data, runtime state, and response allowlists are excluded, and memory is limited to the confirmed none, core, or everything selection.
  • Added cross-user coverage proving that an agent published by Alice is discoverable and importable by another community member, plus update and unpublish coverage.

Checks: 3,448 desktop unit tests; 27 agents E2E tests; relay validation tests; desktop Tauri, Rust, and mobile pre-push suites.

…log-sharing

# Conflicts:
#	desktop/src/features/agents/ui/AgentDefinitionDialog.tsx
#	desktop/src/features/agents/ui/UnifiedAgentsSection.tsx
…log-sharing

# Conflicts:
#	desktop/tests/helpers/bridge.ts
@wpfleger96

Copy link
Copy Markdown
Member

hey @klopez4212 — consolidated review feedback from two independent passes at e2abf33. Two important items, both blocking; the rest minor (flagged for completeness, weigh accordingly). The overall shape of the feature is strong: the relay-side envelope validator is a strict allowlist with unknown-field rejection at every level, catalog fetches verify same-relay origin + size + SHA-256 + full decode before import, and imports mint a fresh keypair consuming zero source identity material. Also, nice catch pushing e2abf33 — that resolved the stale "Create from scratch" locator in global-agent-config-screenshots.spec.ts before we finished reviewing.

1. (important, flagged independently by both reviewers) Catalog-shared agents in allowlist mode are published in a state every community member's import will reject. sanitizeCatalogSnapshotBytes copies definition.respondTo into the public snapshot but strips respondToAllowlist (personaCatalogRelay.ts:406-427), so an allowlist-mode source produces respondTo: "allowlist" with no list. The catalog add path feeds those bytes to confirm_agent_snapshot_import with keepAllowlist: false (usePersonaActions.ts:325-336), and the Rust import boundary hard-rejects allowlist mode with an empty list before minting (snapshot/import.rs:161-169 — "cannot import: no pubkeys to grant access to"). We reproduced the sanitizer output at this head: {"respondTo":"allowlist"} with no allowlist key. Net effect: publish succeeds, the entry is discoverable community-wide, and every "Add agent" click fails. Note the sanitizer's own test pins the broken shape (personaCatalogRelay.test.mjs:143-144 asserts respondTo === "allowlist" survives with the list removed). Fix: when sanitization removes an allowlist-mode list, omit respondTo (import defaults owner-only) or write "owner-only"; update that test and add a sanitize→import round-trip regression.

2. (important, flagged independently by both reviewers) The new integration test agents.spec.ts:363 ("the new agent card offers create, discover, and import") fails deterministically at this head — 3/3 CI attempts and 3/3 focused local attempts, 89.5px vs the <1px bound (CI job 89533647787). The assertion requires the rightmost first-row persona card's right edge to equal the header's right edge, but the grid centers fixed-width auto-fill columns (grid-cols-[repeat(auto-fill,minmax(220px,240px))] justify-center, UnifiedAgentsSection.tsx:63) and the excluded new-agent-card occupies the true rightmost slot — the asserted equality isn't a product invariant. Include the new-agent card in the measured row or assert container/grid alignment instead.

3. (minor) The client-side NIP-33 fallback tie-break reverses the relay's. catalogPublicationsFromEvents sorts equal-second events by descending id and marks the coordinate seen before validating the content (personaCatalogRelay.ts:220-235), while buzz storage defines the live head as the lowest id on equal created_at (crates/buzz-db/src/lib.rs, replaceable-head query orders created_at DESC, id ASC). The live relay path returns one head so impact is limited to the stated older-relay/test-fixture fallback, but that fallback can pick the opposite publish/unpublish head. Sort ids ascending and only mark the coordinate seen after a successful parse; an equal-second test would pin it.

4. (minor) parseCatalogContent accepts any string for agent.avatarUrl (personaCatalogRelay.ts:184). The publish-side publicAvatarUrl guard only protects honest publishers — a hand-crafted event bypasses it, and the relay validator also only requires string-or-null. Snapshot URLs get a strict http(s) + shape check (isSafeSnapshotReference); avatar URLs rendered from other members' events deserve the same scheme guard. Low impact (lands in an img src), defense-in-depth.

Items 1 and 2 are the merge-blockers from our side. CI triage at head for the rest of the red checks: Rust Lint, Desktop Build (macOS), Desktop E2E Relay, Desktop Smoke E2E (2), and the aarch64 cross-compile all died in the activate-hermit setup step with a 504 downloading just from GitHub releases — infra noise, not this PR (cargo fmt --all --check passes locally at this head). The only real CI failure is item 2. Also for what it's worth: agents.spec.ts:1597 ("share access controls include the selected memories") was flaky in the same job (failed, passed on retry #1) — worth a glance while you're in that file, but not blocking.

@klopez4212

Copy link
Copy Markdown
Contributor Author

🤖 @wpfleger96 Addressed in dd22a7c.

  • Catalog sanitization now converts allowlist-mode agents to owner-only when the private response allowlist is removed. The uploaded-byte integration assertion and native import-contract test cover the public snapshot through the import boundary.
  • Equal-second replaceable heads now use the relay canonical lowest-ID tie-break. I intentionally kept the coordinate claimed before parsing so an invalid canonical head fails closed instead of resurrecting an older publication; that behavior now has its own regression test.
  • Catalog avatar URLs now require bounded HTTP(S) values on both publication and discovery.
  • The 89.5px alignment failure was already corrected in b72e024; the focused catalog integration set is green (8/8).

Local checks are green: pnpm check, 3,486 desktop unit tests, 1,628 native desktop tests, focused native import coverage, and 8 focused catalog integration tests. The fresh CI run is in progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants