feat(agents): gate activity-feed ingress by relay ownership#1060
feat(agents): gate activity-feed ingress by relay ownership#1060tellaho wants to merge 4 commits into
Conversation
- Add a relay ownership endpoint backed by agent_owner_pubkey and is_agent_owner so desktop activity visibility uses the same source of truth as observer telemetry authorization. - Add a Tauri ownership command, frontend API wrapper, and useCanViewAgentActivity hook with local managed-agent optimism only while relay ownership is loading. - Replace profile popover, profile panel, and members sidebar activity gates so owned agents can open activity across different builds and worktrees without relying on local managed-agent lists. - Refactor channel agent session candidate resolution so owned agents can keep an activity panel open when channel metadata is stale, while preserving local-only lifecycle controls. - Add focused desktop unit coverage for the ownership predicate and stale-metadata session resolution path, plus E2E bridge support for ownership mocks.
- Extend canViewAgentActivity coverage for relay-backed agent sessions - Harden Tauri ownership resolution and observer relay scoping - Wire updated visibility rules through profile panel and popover surfaces
- Update channel activity candidate resolution to treat ChannelMember.isAgent the same as bot role membership. - Keep agent typing classification and Activity panel scoping aligned so agent typing does not fall through to the generic human typing row. - Add regression coverage for isAgent channel members being created as activity candidates and retained in channel scope.
23fde5c to
0851322
Compare
Resolve conflicts in the agent-session hooks by combining both sides: main's URL/history-backed panel state (agentsLoaded gate, externalized openAgentSessionPubkey) and this PR's relay-ownership stale-session guard. The auto-close effect now waits for agent queries to settle AND keeps the panel open for owners, closing a stale param only when ownership resolves isOwner === false. Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co> Signed-off-by: Wes <wesbillman@users.noreply.github.com>
wpfleger96
left a comment
There was a problem hiding this comment.
Did a thorough review pass with an independent second reviewer. The direction here is right and the relay layer is solid — clean NIP-98 + replay + relay-membership enforcement, and the per-bridge known-pubkey fix in observerRelayStore is a genuine bug fix. Two blocking things at the FE seam, both contradict the relay-authoritative tightening goal and are a handful of lines each, plus a couple of nits. Details inline.
| isOwner: raw.is_owner, | ||
| }; | ||
| } catch (error) { | ||
| const owner = await resolveOaOwner(agentPubkey).catch(() => null); |
There was a problem hiding this comment.
I think this fallback quietly undermines the relay-authoritative goal. On any relay error we fall back to resolveOaOwner and return isOwner = owner.isMe, but those two authorities can disagree: the relay's agent_owner_pubkey is immutable (set_agent_owner only writes WHERE agent_owner_pubkey IS NULL, first-mint-wins), and auth.rs already logs "NIP-OA owner differs from DB owner" as a tolerated case. So if an agent's NIP-OA auth tag gets rotated after first mint, resolveOaOwner reports the new owner as isMe=true while the relay would say is_owner=false — meaning a transient 5xx or network blip silently grants the activity gate to someone the authoritative path denies. Can we fail closed here instead? Return isOwner: false and surface loading/error rather than substituting a divergent authority. If we need the fallback for offline UX, I'd gate it to deny-only — never let it grant.
| const { goChannel } = useAppNavigation(); | ||
| const activeTurns = useActiveAgentTurns(isBot ? pubkey : null); | ||
| const activeTurns = useActiveAgentTurns(pubkey); | ||
| const canShowActivity = canViewActivity || activeTurns.length > 0; |
There was a problem hiding this comment.
Reading this literally, the || activeTurns.length > 0 reopens the ingress for a non-owner whenever the agent has active turns — which cuts against the tightening this PR is doing. I traced it and it doesn't actually leak today, because the active-turns store only gets populated for managed agents, so for a non-owner activeTurns stays empty and the OR collapses to canViewActivity. But that safety lives in a different module with no guard here at the call-site, and it's a new widening vs main (where the ingress gated purely on canViewActivity). A future change that populates turns for observed/non-managed agents would silently reopen it. Can we drop || activeTurns.length > 0 from the visibility decision — it's redundant for owners — or AND it with an explicit local ownership/managed check so the invariant is visible right here?
| const userStatus = userStatusQuery.data?.[pubkey.toLowerCase()]; | ||
| const activeTurns = useActiveAgentTurns(role === "bot" ? pubkey : null); | ||
| const activeTurns = useActiveAgentTurns(pubkey); | ||
| const canShowActivity = canViewActivity || activeTurns.length > 0; |
There was a problem hiding this comment.
Same || activeTurns.length > 0 widening as in UserProfilePanelSections.tsx — flagging separately because this call-site is a bit more exposed: it reads useActiveAgentTurns(pubkey) directly with no local bridge, so it leans entirely on the global store staying owner-scoped. Same fix: drop the OR from the visibility decision, or AND it with a local ownership check.
| ) ?? | ||
| allAgentCandidates.find( | ||
| (agent) => normalizePubkey(agent.pubkey) === normalized, | ||
| ) ?? { |
There was a problem hiding this comment.
Minor: this fabricates a synthetic relay-agent for any openAgentSessionPubkey that doesn't match a candidate, where the old ChannelPane returned null. It's safe today — hasObserver is false for a synthetic agent so no transcript loads, and the auto-close effect closes non-owner sessions once ownership resolves — but it means the panel briefly mounts for any pubkey, including bogus/stale ones. Worth a one-line comment noting the auto-close backstop is what makes this safe, so the next reader doesn't have to reconstruct it.
| .map_err(|e| internal_error(&format!("ownership lookup failed: {e}")))? | ||
| .and_then(|(_policy, owner)| owner); | ||
|
|
||
| let is_owner = state |
There was a problem hiding this comment.
Nit: get_agent_channel_policy already returned owner just above, so is_owner could be derived as owner_pubkey == Some(actor_bytes) instead of a second query. Totally minor — the SQL is parameterized and indexed and the comment on is_agent_owner justifies the narrower query — just flagging for minimalism. Fine to leave as-is.
Category: improvement
User Impact: Agent activity in the desktop Activity panel now consistently shows up and respects relay ownership, so people only see activity they are entitled to.
Problem: Activity ingress was not reliably gated by ownership, and agent members were not classified for ingress — activity could surface inconsistently or to the wrong viewers.
Solution: The ingress + ownership foundation: relay-side ownership gating, refined visibility ownership checks, and agent-member classification for activity ingress (relay
.rs+ Tauri ownership commands + client viewer hooks), backed by ownership-resolution and agent-session-candidate tests.Commits
feat(agents): gate activity by relay ownershipfeat(agents): refine activity visibility ownership checksfix(agents): classify agent members for activity ingressReproduction steps
🤖 Branch split + PR by Ned (sequencer) / Bart (builder).