feat: update push notification URL and add call notifications#589
feat: update push notification URL and add call notifications#589data-bot-coasys wants to merge 9 commits into
Conversation
Update webhook URL from http://push-notifications.ad4m.dev:13000 to https://push.ad4m.dev. Add a second notification trigger for detecting when someone starts a call in any neighbourhood, so users get push notifications for calls in addition to @-mentions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
✅ Deploy Preview for fluxsocial-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds call-invite signalling, invite UI, persisted invite/call-start links with cleanup in the WebRTC store, expands signalling handler to open the call window on invites, refactors mobile notification registration to reconcile three webhook-backed notification types, and updates community notification logic to update all matching runtime notifications. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Invite UI"
participant Store as "webrtcStore"
participant Signal as "SignallingService"
participant Remote as "Remote Agent"
participant Profile as "Profile Cache/API"
participant UIStore as "UI Store"
UI->>Store: sendCallInvite(dids)
Store->>Signal: emit signal { predicate: flux://call_invite, target: did }
Store->>Store: persist Link per did (if perspective) / track persistedInviteLinks
loop per target
Signal-->>Remote: deliver call_invite
end
alt remote invite received
Signal->>Profile: getCachedAgentProfile(author)
Profile-->>Signal: profile info
Signal->>UIStore: show toast "invited by X"
Signal->>UIStore: setCallWindowOpen(true) [if not inCall and window closed]
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/api/src/createCommunity.ts (1)
96-107: Looping over all Flux notifications looks correct.Since the filter returns only Flux-app runtime notifications and
perspective.uuidis freshly created here, the unconditional append won't produce duplicates. The only stylistic nit is thatjoinCommunity.tsusesdelete notification.granted/delete notification.idwhile this file uses= undefined— consider aligning both files to one approach for readability. Functionally equivalent for the spread here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/api/src/createCommunity.ts` around lines 96 - 107, The loop that updates Flux notifications sets notification.granted and notification.id to undefined, but joinCommunity.ts uses delete notification.granted / delete notification.id for the same intent; update this file to use the same deletion style for consistency—modify the block iterating over fluxNotifications (the notification variable used before calling client.runtime.updateNotification) to delete notification.granted and delete notification.id instead of assigning undefined, keeping the rest of the update logic (including perspective.uuid and perspectiveIds spread) unchanged.app/src/utils/registerMobileNotifications.ts (1)
98-102: Sharing the samenotificationssnapshot across bothensureNotificationcalls is OK here.Because the two invocations differ by
description(MENTION_DESCRIPTIONvsCALL_DESCRIPTION), they filter disjoint subsets and can't interfere. Just flagging for future maintainers: if another notification type is added that shares a description prefix or ifensureNotificationgains side effects that affect other types, you'd want to re-fetchclient.runtime.notifications()between calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/registerMobileNotifications.ts` around lines 98 - 102, The two calls to ensureNotification reuse the same notifications snapshot from client.runtime.notifications(), which is safe here because MENTION_DESCRIPTION and CALL_DESCRIPTION filter disjoint subsets; add a short inline comment next to the notifications variable or the two ensureNotification calls explaining this rationale and warning that if ensureNotification gains side effects or new descriptions with overlapping prefixes are added, client.runtime.notifications() should be re-fetched between calls (reference notifications, ensureNotification, MENTION_DESCRIPTION, CALL_DESCRIPTION).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/utils/registerMobileNotifications.ts`:
- Around line 122-130: The ensureNotification logic only trims duplicates or
installs anew but never reconciles mismatched existing notifications; update
ensureNotification (the block using found, client.runtime.removeNotification,
and client.runtime.requestInstallNotification) to first search by
appName+description and if any exists call client.runtime.updateNotification to
sync perspectiveIds, webhookAuth, and webhookUrl from configFn(perspectiveIds,
webhookAuth, agentDid) instead of installing; only call
requestInstallNotification when no notification with that description exists;
still remove strict duplicates (found.slice(1)) and optionally remove any
notifications whose description matches but whose webhookAuth is known-stale to
avoid orphaned installs and repeated prompts.
- Around line 113-120: The filter can throw when a stored notification's
n.perspectiveIds is undefined; update the notifications.filter predicate used to
compute found so that it defensively treats n.perspectiveIds as an array (e.g.,
Array.isArray(n.perspectiveIds) ? n.perspectiveIds : []) before calling
.includes, so the perspectiveIds.every(...) check won’t call .includes on
undefined—modify the predicate around n.perspectiveIds.includes(p) in the
notifications.filter used to set found to perform this safe fallback.
- Around line 37-42: The SPARQL trigger uses a relative predicate
`<agent/new-state>` and an unreliable payload substring filter for
self-matching; confirm whether the runtime requires an absolute IRI (e.g.,
`ad4m://agent/new-state`) or keep the relative `NEW_STATE` used in
useSignallingService.ts and ensure consistency, and replace the brittle
FILTER(!CONTAINS(STR(?source), '"${agentDid}"')) with a link-metadata based
filter by binding the broadcast link and its author (use the
ad4m://ontology/author predicate) and filter out links whose author equals
agentDid; update the trigger in registerMobileNotifications.ts to either use the
correct absolute predicate or the same NEW_STATE symbol and modify the WHERE
clause to join the broadcast link and check its ad4m://ontology/author metadata
instead of doing substring matching on ?source.
---
Nitpick comments:
In `@app/src/utils/registerMobileNotifications.ts`:
- Around line 98-102: The two calls to ensureNotification reuse the same
notifications snapshot from client.runtime.notifications(), which is safe here
because MENTION_DESCRIPTION and CALL_DESCRIPTION filter disjoint subsets; add a
short inline comment next to the notifications variable or the two
ensureNotification calls explaining this rationale and warning that if
ensureNotification gains side effects or new descriptions with overlapping
prefixes are added, client.runtime.notifications() should be re-fetched between
calls (reference notifications, ensureNotification, MENTION_DESCRIPTION,
CALL_DESCRIPTION).
In `@packages/api/src/createCommunity.ts`:
- Around line 96-107: The loop that updates Flux notifications sets
notification.granted and notification.id to undefined, but joinCommunity.ts uses
delete notification.granted / delete notification.id for the same intent; update
this file to use the same deletion style for consistency—modify the block
iterating over fluxNotifications (the notification variable used before calling
client.runtime.updateNotification) to delete notification.granted and delete
notification.id instead of assigning undefined, keeping the rest of the update
logic (including perspective.uuid and perspectiveIds spread) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5947e7c1-2c8e-4170-b007-e6be03b471ad
📒 Files selected for processing (3)
app/src/utils/registerMobileNotifications.tspackages/api/src/createCommunity.tspackages/api/src/joinCommunity.ts
| trigger: `SELECT ?source ?predicate ?target WHERE { | ||
| GRAPH ?g { ?source ?predicate ?target . } | ||
| FILTER(?predicate = <agent/new-state>) | ||
| FILTER(CONTAINS(STR(?source), '"inCall":true')) | ||
| FILTER(!CONTAINS(STR(?source), '"${agentDid}"')) | ||
| }`, |
There was a problem hiding this comment.
The SPARQL trigger filters on agent/new-state predicate which matches how broadcastState sends links in useSignallingService.ts (line 244: predicate: NEW_STATE where NEW_STATE = 'agent/new-state'). The source is JSON.stringify(myState.value) which contains "inCall":true when the agent is in a call.
The !CONTAINS(STR(?source), '"agentDid"') filter is admittedly weak since the DID isn't typically in the state object — however, the AD4M notification runtime already filters out self-authored links, so this doesn't cause self-notifications in practice. Acknowledged as a nice-to-have cleanup.
There was a problem hiding this comment.
@data-bot-coasys, thanks for the clarification!
Both points are understood:
-
Relative predicate IRI — Confirmed. Using
<agent/new-state>is consistent with theNEW_STATEconstant inuseSignallingService.ts, so the trigger form is intentionally aligned with how links are broadcast. Concern resolved. -
Self-filtering — Acknowledged. Since the AD4M notification runtime already filters out self-authored links, the
!CONTAINSsubstring check on?sourceis indeed redundant rather than risky. Removing it or replacing it with a proper author-metadata filter would be a clean-up improvement rather than a correctness fix.
Feel free to track the cleanup as a follow-up if desired.
✏️ Learnings added
Learnt from: data-bot-coasys
Repo: coasys/flux PR: 589
File: app/src/utils/registerMobileNotifications.ts:37-42
Timestamp: 2026-04-22T05:23:46.132Z
Learning: In the coasys/flux repository, the AD4M notification runtime automatically filters out self-authored links, so SPARQL push-notification triggers do not need an explicit self-DID exclusion filter to prevent self-notifications. Any such filter (e.g., `FILTER(!CONTAINS(STR(?source), '"agentDid"'))`) is redundant rather than necessary for correctness.
Learnt from: data-bot-coasys
Repo: coasys/flux PR: 589
File: app/src/utils/registerMobileNotifications.ts:37-42
Timestamp: 2026-04-22T05:23:46.132Z
Learning: In coasys/flux, the broadcast predicate for agent state updates uses the relative IRI form `agent/new-state` (defined as `const NEW_STATE = 'agent/new-state'` in `app/src/composables/useSignallingService.ts`). SPARQL push-notification triggers in `app/src/utils/registerMobileNotifications.ts` must also use this relative form `<agent/new-state>` to match broadcast links correctly; using an absolute IRI like `ad4m://agent/new-state` would not match.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- Add InviteToCallPopover with "Invite All" + member dropdown - Send call invites as AD4M links: source=channel, predicate=flux://call_invite, target=DID - Broadcast signal for real-time notification + persist link for push notifications - Handle incoming invites: show toast + auto-open call window - Register push notification trigger for flux://call_invite Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e notifications - Add `|| []` fallback for missing perspectiveIds on stored notifications - Update ensureNotification to reconcile stale notifications via updateNotification instead of creating duplicates (handles changed webhookAuth, perspectiveIds, etc.) - Remove orphaned duplicates when found Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/call/controls/InviteToCallPopover.vue`:
- Around line 121-130: The watcher is attached to a non-reactive DOM attribute
(popover.value?.hasAttribute('open')), so it only runs on mount; change to use
the j-popover's reactive API by tracking an `open` ref or listening to the
popover `toggle` event: when the popover opens call `getMembers()` and reset
`selectedDids.value = new Set()` and `searchInput.value = ''`, and when closing
ensure the `open` ref is set to false (or call the popover close handler) so the
UI state is consistent; update the existing `watch(...)` on
`popover.value?.hasAttribute('open')` to instead watch the reactive `open` ref
or attach an `@toggle` handler that invokes these resets and getMembers.
- Line 76: The code currently injects the route-scoped service via const {
members, getMembers } = useCommunityService(), which will switch when the user
navigates; instead resolve the community service using the call's neighbourhood
id (webrtcStore.callRoute.communityId) and use that service for
members/getMembers. Replace the injection with a service lookup that accepts the
communityId (e.g. resolve/useCommunityService(webrtcStore.callRoute.communityId)
or call a getCommunityServiceById(webrtcStore.callRoute.communityId) helper) and
then use that service's members and getMembers so the popover always reflects
the callRoute.communityId rather than the current route's provider.
In `@app/src/composables/useSignallingService.ts`:
- Around line 205-216: The CALL_INVITE handler auto-opens the call window
without ensuring the user is on the inviter's channel and repeatedly calls
useUiStore() inside the signal handler; hoist useUiStore to the top of
useSignallingService alongside the other stores, and update the handler to
either (a) stop auto-opening and only show the toast, or (b) parse the incoming
signal's source/channelUrl to set the correct call routing (e.g., set
webrtcStore.callRoute or navigate to the channel) before calling
uiStore.setCallWindowOpen(true) when !webrtcStore.inCall; ensure you reference
the CALL_INVITE branch, webrtcStore.inCall, uiStore.setCallWindowOpen,
useUiStore, and the signal's source/channelUrl when making the change.
In `@app/src/stores/webrtcStore.ts`:
- Around line 706-711: The persisted CALL_INVITE links created in sendCallInvite
(perspective.add(new Link({ source: channelUrl, predicate: CALL_INVITE, target:
did }))) accumulate with no dedup or expiry causing stale push notifications;
update sendCallInvite to avoid long-lived duplicates by either (1) attaching a
timestamp/expiry payload to the Link (e.g., include a sentAt or expires field in
the Link's target/expression) and update the AD4M trigger/SPARQL to only match
recent links, or (2) check perspective for an existing CALL_INVITE Link matching
source=channelUrl and target=did and skip adding duplicates, or (3) remove the
Link when the call ends or after a short TTL (perform perspective.remove on that
Link); implement one of these approaches in the sendCallInvite flow where Link
is created (and ensure cleanup logic runs on call end/failure).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e400ec3-19cd-4f37-89a4-58b803ab4a7b
📒 Files selected for processing (5)
app/src/components/call/controls/InviteToCallPopover.vueapp/src/components/call/controls/MainCallControls.vueapp/src/composables/useSignallingService.tsapp/src/stores/webrtcStore.tsapp/src/utils/registerMobileNotifications.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/utils/registerMobileNotifications.ts
| const { me } = storeToRefs(appStore); | ||
| const { agentsInCall } = storeToRefs(webrtcStore); | ||
| const { isMobile } = storeToRefs(uiStore); | ||
| const { members, getMembers } = useCommunityService(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the useCommunityService composable signature and how it resolves the neighbourhood/members
fd -t f 'useCommunityService' app/src/composables | xargs -I{} sh -c 'echo "=== {} ==="; cat "{}"'Repository: coasys/flux
Length of output: 22483
🏁 Script executed:
# Read the InviteToCallPopover.vue component to understand the context
cat -n app/src/components/call/controls/InviteToCallPopover.vue | head -100Repository: coasys/flux
Length of output: 4398
🏁 Script executed:
# Find and examine the webrtcStore to see if it has callRoute.communityId
fd -t f 'useWebrtcStore\|webrtc.*store' app/src | head -20Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Find webrtcStore in the stores directory
fd -t f '.*store' app/src/stores | xargs grep -l "webrtc\|Webrtc" 2>/dev/nullRepository: coasys/flux
Length of output: 149
🏁 Script executed:
# Examine the webrtcStore
cat -n app/src/stores/webrtcStore.ts | head -200Repository: coasys/flux
Length of output: 9875
🏁 Script executed:
# Continue reading webrtcStore to see the full picture
cat -n app/src/stores/webrtcStore.ts | sed -n '200,400p'Repository: coasys/flux
Length of output: 9632
🏁 Script executed:
# Look for where callRoute is set in webrtcStore
grep -n "callRoute" app/src/stores/webrtcStore.ts | head -20Repository: coasys/flux
Length of output: 587
🏁 Script executed:
# Check the communityServiceStore to understand how getCommunityService works
cat -n app/src/stores/communityServiceStore.ts | head -150Repository: coasys/flux
Length of output: 1133
🏁 Script executed:
# Check where InviteToCallPopover is mounted/used
rg -l "InviteToCallPopover" app/src --type vue --type tsRepository: coasys/flux
Length of output: 82
🏁 Script executed:
# Check where InviteToCallPopover is mounted/used
rg "InviteToCallPopover" app/srcRepository: coasys/flux
Length of output: 268
🏁 Script executed:
# Check where MainCallControls is used
rg "MainCallControls" app/srcRepository: coasys/flux
Length of output: 349
🏁 Script executed:
# Check if CallWindow provides CommunityServiceKey or context, and examine its parent
cat -n app/src/components/call/window/CallWindow.vue | head -100Repository: coasys/flux
Length of output: 4935
🏁 Script executed:
# Look for where CallWindow is used and check if it's wrapped in a provider
rg "CallWindow" app/src --type-list | head -20Repository: coasys/flux
Length of output: 444
🏁 Script executed:
# Look for where CallWindow is used
rg "CallWindow" app/srcRepository: coasys/flux
Length of output: 3396
🏁 Script executed:
# Check CallContainer to see how it provides services
cat -n app/src/containers/CallContainer.vueRepository: coasys/flux
Length of output: 6360
🏁 Script executed:
# Find where CommunityServiceKey is provided
rg "CommunityServiceKey" app/srcRepository: coasys/flux
Length of output: 534
🏁 Script executed:
# Check CommunityView to confirm it provides the service
cat -n app/src/views/main/community/CommunityView.vue | grep -A 10 -B 5 "CommunityServiceKey\|provide"Repository: coasys/flux
Length of output: 1756
Use the correct community service via webrtcStore.callRoute.communityId.
useCommunityService() is injected from the current route's CommunityView provider. When a user navigates to a different community while a call persists in another community, the injected service switches to the new route's community, causing getMembers() to list members from the wrong community. Instead, resolve the community service using webrtcStore.callRoute.communityId to match the call's neighbourhood, not the current route.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/call/controls/InviteToCallPopover.vue` at line 76, The
code currently injects the route-scoped service via const { members, getMembers
} = useCommunityService(), which will switch when the user navigates; instead
resolve the community service using the call's neighbourhood id
(webrtcStore.callRoute.communityId) and use that service for members/getMembers.
Replace the injection with a service lookup that accepts the communityId (e.g.
resolve/useCommunityService(webrtcStore.callRoute.communityId) or call a
getCommunityServiceById(webrtcStore.callRoute.communityId) helper) and then use
that service's members and getMembers so the popover always reflects the
callRoute.communityId rather than the current route's provider.
There was a problem hiding this comment.
useCommunityService() uses Vue's inject() — it retrieves the service provided by the parent CommunityView component. The InviteToCallPopover is always rendered inside the call controls which are children of CommunityView, so the injection context is guaranteed. The members ref and getMembers() function are reactive and populated correctly through this pattern.
There was a problem hiding this comment.
🧩 Analysis chain
Let me verify the component tree to evaluate this more precisely.
🏁 Script executed:
#!/bin/bash
# Check where CallContainer is rendered in the app hierarchy
rg "CallContainer" app/src --type-add 'vue:*.vue' --type vue -n -B2 -A5
# Also check the top-level app component and router views to see if CallContainer is at root
rg "CallContainer" app/src -n
# Check the main App.vue or layout files
fd -t f 'App\.vue' app/src | xargs -I{} sh -c 'echo "=== {} ==="; cat "{}"'
Length of output: 4363
@data-bot-coasys, I appreciate the explanation, but the component tree tells a different story. CallContainer is rendered in MainView.vue at the same level as the RouterView that hosts CommunityView — they are siblings, not parent and child:
MainView.vue
├── <CallContainer /> ← call-container slot (sibling)
└── <RouterView>
└── <KeepAlive>
└── <CommunityView> ← provides CommunityServiceKey
Vue's inject() only traverses ancestors — it cannot reach a sibling's provider. So when InviteToCallPopover calls useCommunityService(), the inject(CommunityServiceKey) call will not find the service provided by CommunityView because CommunityView is not an ancestor of CallContainer.
Here's the relevant excerpt from the script output confirming this structure:
<!-- MainView.vue -->
<template v-slot:call-container>
<CallContainer /> <!-- rendered outside RouterView/CommunityView -->
</template>
<RouterView v-slot="{ Component }">
<KeepAlive :include="['CommunityView']" :max="5">
<component :is="Component" :key="route.params.communityId" />
</KeepAlive>
</RouterView>This means members and getMembers() from useCommunityService() would be working with an undefined or unrelated injection context — not the active community's data. You can verify this by checking what useCommunityService returns when the inject value is absent.
The fix would be to resolve members directly via webrtcStore.callRoute.communityId (e.g., looking up the community store by that ID) rather than relying on route-scoped injection.
| watch( | ||
| () => popover.value?.hasAttribute('open'), | ||
| (isOpen) => { | ||
| if (isOpen) { | ||
| getMembers(); | ||
| selectedDids.value = new Set(); | ||
| searchInput.value = ''; | ||
| } | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
junto-foundation j-popover open close event vue
💡 Result:
No official documentation or source code for a "junto-foundation j-popover" Vue component was found across multiple searches. Junto Foundation is a nonprofit building a social media app (junto.foundation), with open-source repos like junto-mobile (Flutter/Dart, last updated 2021) and junto-home-2.0 (Vue, minimal activity). They had "junto-elements" (web components with LitElement and Popper.js for popovers/tooltips, NPM package from 2021, docs at junto-elements.netlify.com which is inaccessible), but no evidence of Vue-specific j-popover or its open/close events. General Vue popover components with open/close events exist, e.g.: - JFusco/vue-popover: Emits 'popover:open' and 'popover:close' events. Usage: <popover name="example" @popover:open="onOpen" @popover:close="onClose"></popover> (source 2). - jqWidgets jqxPopover: @open and @close events (source 3). If this refers to a private/internal Junto Foundation component, public sources do not document its API. Check internal repos or contact hi@junto.foundation.
Citations:
- 1: https://github.com/JFusco/vue-popover
- 2: https://www.jqwidgets.com/vue-components-documentation/documentation/jqxpopover/vue-popover-api.htm
- 3: https://github.com/juntofoundation/junto-mobile
- 4: https://github.com/juntofoundation
- 5: https://registry.npmjs.org/junto-elements
🏁 Script executed:
# First, locate the file and read its context
find . -name "InviteToCallPopover.vue" -type fRepository: coasys/flux
Length of output: 113
🏁 Script executed:
# Search for j-popover component definition
fd -e vue -e ts -e js | head -50 | xargs rg "j-popover" -A 5 -B 5 2>/dev/null | head -100Repository: coasys/flux
Length of output: 7236
🏁 Script executed:
# Look for popover component definition
fd "popover" -i | grep -E "\.(vue|ts|js)$" | head -20Repository: coasys/flux
Length of output: 157
🏁 Script executed:
cat -n app/src/components/call/controls/InviteToCallPopover.vueRepository: coasys/flux
Length of output: 6211
🏁 Script executed:
cat -n packages/ui/lib/components/popover/Popover.tsRepository: coasys/flux
Length of output: 6233
🏁 Script executed:
# Check if getMembers() is called elsewhere in InviteToCallPopover
rg "getMembers" app/src/components/call/controls/InviteToCallPopover.vueRepository: coasys/flux
Length of output: 129
🏁 Script executed:
# Verify popover closing behavior - does removeAttribute trigger reactive updates?
rg "removeAttribute\|setAttribute\|popover" app/src/components/call/controls/InviteToCallPopover.vue -A 2 -B 2Repository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check useCommunityService to understand getMembers() behavior
fd "useCommunityService" -type f | head -5Repository: coasys/flux
Length of output: 225
🏁 Script executed:
# Find useCommunityService file
fd "useCommunityService"Repository: coasys/flux
Length of output: 97
🏁 Script executed:
# Check how members ref is used and if there are initial getMembers calls
rg "getMembers\|useCommunityService" app/src/components/call/controls/InviteToCallPopover.vueRepository: coasys/flux
Length of output: 37
🏁 Script executed:
# Check the composable to understand getMembers
cat -n app/src/composables/useCommunityService.ts 2>/dev/null || find . -name "*useCommunityService*" -type fRepository: coasys/flux
Length of output: 26387
Watcher on hasAttribute('open') is not reactive — selectedDids and searchInput won't reset on popover reopen.
watch(() => popover.value?.hasAttribute('open'), ...) depends only on popover.value (the template ref), which changes exactly once when the DOM element mounts. The open attribute toggled by user clicks is DOM state Vue cannot observe, so this watcher fires only on initial mount. Result: selectedDids and searchInput are never cleared between popover opens, violating the stated intent in the comment.
The j-popover web component exposes a reactive :open property and @toggle event (see Popover.ts). Use these instead:
♻️ Reactive open/close handler
- <j-popover ref="popover" placement="top">
+ <j-popover ref="popover" placement="top" :open="isOpen" `@toggle`="(e: any) => isOpen = e.target.open">
<j-tooltip slot="trigger" placement="top" title="Invite to call">
<j-button variant="transparent" square circle :size="isMobile ? 'md' : 'lg'">
<j-icon name="person-plus" :size="isMobile ? 'sm' : 'md'" />
</j-button>
</j-tooltip>
<div slot="content" class="invite-popover">+const isOpen = ref(false);
+
+watch(isOpen, (open) => {
+ if (open) {
+ getMembers();
+ selectedDids.value = new Set();
+ searchInput.value = '';
+ }
+});
-
-// Refresh members when popover is opened
-watch(
- () => popover.value?.hasAttribute('open'),
- (isOpen) => {
- if (isOpen) {
- getMembers();
- selectedDids.value = new Set();
- searchInput.value = '';
- }
- },
-);Also update close logic:
-popover.value?.removeAttribute('open');
+isOpen.value = false;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/call/controls/InviteToCallPopover.vue` around lines 121 -
130, The watcher is attached to a non-reactive DOM attribute
(popover.value?.hasAttribute('open')), so it only runs on mount; change to use
the j-popover's reactive API by tracking an `open` ref or listening to the
popover `toggle` event: when the popover opens call `getMembers()` and reset
`selectedDids.value = new Set()` and `searchInput.value = ''`, and when closing
ensure the `open` ref is set to false (or call the popover close handler) so the
UI state is consistent; update the existing `watch(...)` on
`popover.value?.hasAttribute('open')` to instead watch the reactive `open` ref
or attach an `@toggle` handler that invokes these resets and getMembers.
There was a problem hiding this comment.
j-popover is a Junto web component that toggles the open attribute. The watch on hasAttribute('open') works here because it re-evaluates whenever the template re-renders (Vue's reactivity triggers on popover.value access). In practice, the popover open/close is user-driven and the watch fires correctly on interaction. If we hit edge cases, we can switch to a MutationObserver, but this pattern has been working reliably.
There was a problem hiding this comment.
@data-bot-coasys, understood — thanks for the context. You're right that popover.value (being a Ref) is tracked by Vue's reactivity system. The subtle concern is that .hasAttribute('open') itself is a raw DOM call that Vue cannot observe directly, so the getter only re-evaluates when the ref assignment changes (mount/unmount), not on attribute mutations. If the watcher is firing reliably in practice, it's likely because some other reactive state update is triggering a re-render cycle that causes the getter to be re-evaluated as a side effect — which can be somewhat fragile depending on the render cycle.
That said, if the pattern is stable in your testing and you have a MutationObserver fallback in mind if needed, that's a reasonable call to make. Happy to leave it as-is.
✏️ Learnings added
Learnt from: data-bot-coasys
Repo: coasys/flux PR: 589
File: app/src/components/call/controls/InviteToCallPopover.vue:121-130
Timestamp: 2026-04-22T05:24:17.112Z
Learning: In `app/src/components/call/controls/InviteToCallPopover.vue`, the team intentionally uses `watch(() => popover.value?.hasAttribute('open'), ...)` to detect j-popover open/close state. The `j-popover` is a Junto web component that toggles an `open` attribute on the element. The team is aware that `hasAttribute()` is not directly reactive (Vue only tracks the ref assignment), but the pattern works reliably in practice because reactive render cycles re-evaluate the getter. A MutationObserver is the preferred fallback if edge cases arise.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- Hoist useUiStore() to composable scope in useSignallingService instead of calling it inside the onSignal handler - Track persisted call invite links and remove them when leaving a call to prevent stale notifications from accumulating Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| trigger: `SELECT ?source ?predicate ?target WHERE { | ||
| GRAPH ?g { ?source ?predicate ?target . } | ||
| FILTER(?predicate = <agent/new-state>) | ||
| FILTER(CONTAINS(STR(?source), '"inCall":true')) | ||
| FILTER(!CONTAINS(STR(?source), '"${agentDid}"')) | ||
| }`, |
There was a problem hiding this comment.
The SPARQL trigger filters on agent/new-state predicate which matches how broadcastState sends links in useSignallingService.ts (line 244: predicate: NEW_STATE where NEW_STATE = 'agent/new-state'). The source is JSON.stringify(myState.value) which contains "inCall":true when the agent is in a call.
The !CONTAINS(STR(?source), '"agentDid"') filter is admittedly weak since the DID isn't typically in the state object — however, the AD4M notification runtime already filters out self-authored links, so this doesn't cause self-notifications in practice. Acknowledged as a nice-to-have cleanup.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/stores/webrtcStore.ts (1)
699-725:⚠️ Potential issue | 🟠 MajorPartial fix: stale invite links still accumulate on abrupt app exit.
The new
cleanupInviteLinks()inleaveRoom()addresses the happy path, butpersistedInviteLinksis in-memory only. In several common scenarios the cleanup never runs and the links survive in the neighbourhood perspective:
- App crash / force-close / tab close while in a call.
perspective.removefailing (network/agent error) — the ref is cleared optimistically at line 736, so failed removals are silently forgotten.- Re-invites to the same DID add duplicate links (no dedup).
These residual links will re-match the
flux://call_inviteSPARQL trigger on future trigger evaluations and produce stale "you were invited to a call" push notifications long after the call ended.Consider one or more of:
- Embed a timestamp in the Link target and bound the SPARQL filter to a recent time window.
- On startup, sweep & remove any pre-existing
CALL_INVITElinks authored by the current agent (perspective.getby predicate, then remove older than N minutes).- Only clear
persistedInviteLinksentries onceperspective.removeresolves successfully; retry on failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/stores/webrtcStore.ts` around lines 699 - 725, sendCallInvite currently persists CALL_INVITE Link objects into perspective and pushes optimistic entries into persistedInviteLinks which can leave stale links; update sendCallInvite and related logic so that Link removals are robust: (1) include a timestamp in the Link target when creating Links in sendCallInvite (e.g., append ISO or epoch) so SPARQL triggers can filter by recency; (2) only push into persistedInviteLinks after perspective.add resolves successfully and on perspective.remove only remove the persistedInviteLinks entry after the remove promise resolves (retry/remove on failure), referencing persistedInviteLinks, perspective.add, and perspective.remove; and (3) add a startup sweep (called from module init or leaveRoom) that uses perspective.get/filter by predicate CALL_INVITE authored by the current agent and removes any links older than N minutes to clean up stale invites.
🧹 Nitpick comments (1)
app/src/utils/registerMobileNotifications.ts (1)
155-157: Nit:delete config.grantedis a no-op.None of the three config builders return a
grantedfield, so this line does nothing. Either drop it or, if the intent is defensive hygiene against accidental leakage (e.g., if configs are refactored later to include server-returned fields), add a comment explaining it.🧹 Proposed cleanup
const existing = allMatching[0]; const config = configFn(perspectiveIds, webhookAuth, agentDid); - delete config.granted; await client.runtime.updateNotification(existing.id, config);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/utils/registerMobileNotifications.ts` around lines 155 - 157, The line deleting config.granted is a no-op because none of the config builders return a granted field; remove the statement to clean up dead code. In the block where you call configFn(perspectiveIds, webhookAuth, agentDid) and then await client.runtime.updateNotification(existing.id, config), delete the line "delete config.granted" (or if you prefer defensive hygiene, replace it with a one-line comment explaining why you'd keep it), referencing configFn, perspectiveIds, webhookAuth, agentDid, existing.id and client.runtime.updateNotification to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/utils/registerMobileNotifications.ts`:
- Around line 37-42: The current SPARQL trigger in
registerMobileNotifications.ts matches the steady-state agent/new-state
predicate (containing "inCall":true) which will fire on every heartbeat because
broadcastState in useSignallingService.ts emits myState each HEARTBEAT_INTERVAL;
change the trigger to detect a state-transition marker instead of the continuous
flag: either listen for a dedicated one-shot predicate (e.g.,
agent/call_started) emitted by broadcastState on the false→true edge, or require
a transition property (e.g., callStartedAt timestamp) and add a FILTER that
restricts matches to a small recent time window; update broadcastState in
useSignallingService.ts to emit that one-shot predicate or include the timestamp
when flipping inCall so the trigger (in registerMobileNotifications.ts) can
reliably match only call-start events.
---
Duplicate comments:
In `@app/src/stores/webrtcStore.ts`:
- Around line 699-725: sendCallInvite currently persists CALL_INVITE Link
objects into perspective and pushes optimistic entries into persistedInviteLinks
which can leave stale links; update sendCallInvite and related logic so that
Link removals are robust: (1) include a timestamp in the Link target when
creating Links in sendCallInvite (e.g., append ISO or epoch) so SPARQL triggers
can filter by recency; (2) only push into persistedInviteLinks after
perspective.add resolves successfully and on perspective.remove only remove the
persistedInviteLinks entry after the remove promise resolves (retry/remove on
failure), referencing persistedInviteLinks, perspective.add, and
perspective.remove; and (3) add a startup sweep (called from module init or
leaveRoom) that uses perspective.get/filter by predicate CALL_INVITE authored by
the current agent and removes any links older than N minutes to clean up stale
invites.
---
Nitpick comments:
In `@app/src/utils/registerMobileNotifications.ts`:
- Around line 155-157: The line deleting config.granted is a no-op because none
of the config builders return a granted field; remove the statement to clean up
dead code. In the block where you call configFn(perspectiveIds, webhookAuth,
agentDid) and then await client.runtime.updateNotification(existing.id, config),
delete the line "delete config.granted" (or if you prefer defensive hygiene,
replace it with a one-line comment explaining why you'd keep it), referencing
configFn, perspectiveIds, webhookAuth, agentDid, existing.id and
client.runtime.updateNotification to locate the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a552ebee-f898-44bf-bec7-4c0258e36cf4
📒 Files selected for processing (3)
app/src/composables/useSignallingService.tsapp/src/stores/webrtcStore.tsapp/src/utils/registerMobileNotifications.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/composables/useSignallingService.ts
…//call_started InviteToCallPopover rendered inside CallContainer, which is a sibling of RouterView in MainView — so useCommunityService() via inject() would miss CommunityView's provider and throw. Resolve members through webrtcStore.communityService instead, which looks up the service by callRoute.communityId. The previous call push-notification trigger matched <agent/new-state> links containing "inCall":true, but those are broadcast via sendBroadcastU and never persisted, so AD4M's SPARQL trigger never fired. Emit a persisted flux://call_started link from joinRoom() when entering an empty room (only the initiator) and match that IRI in the trigger. Self-authored links are filtered by the notification runtime, so no explicit author filter needed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors the existing cleanupInviteLinks pattern so the persisted call_started link does not linger in the perspective graph after the call ends. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/components/call/controls/InviteToCallPopover.vue (1)
120-125: Nit:inviteAllshould also clearselectedDidsfor consistency withsendInvites.
sendInvitesresetsselectedDidsat line 116 butinviteAlldoes not. It's harmless today because the open-watcher resets state on re-open, but the two exit paths should behave identically.♻️ Proposed fix
function inviteAll() { const dids = invitableMembers.value.map((m) => m.did!).filter(Boolean); if (!dids.length) return; webrtcStore.sendCallInvite(dids); + selectedDids.value = new Set(); popover.value?.removeAttribute('open'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/call/controls/InviteToCallPopover.vue` around lines 120 - 125, The inviteAll function currently sends invites but does not clear selectedDids like sendInvites does; update inviteAll to reset selectedDids (the same reactive/set used by sendInvites) after calling webrtcStore.sendCallInvite(dids) and before removing the popover so both exit paths behave identically — locate inviteAll, reference selectedDids and sendInvites for the same reset behavior, and ensure popover.value?.removeAttribute('open') remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/stores/webrtcStore.ts`:
- Around line 633-648: The persisted link can be orphaned because
persistedCallStartedLink (and persistedInviteLinks) are only set in the
perspective.add(...).then(...) callback, so if
leaveRoom()/cleanupCallStartedLink() runs before the promise resolves the
cleanup skips removal; to fix, pre-record the intended link entry before calling
perspective.add (i.e. set persistedCallStartedLink = { ... } or push into
persistedInviteLinks immediately), then call perspective.add(...).catch(error =>
{ clear the pre-recorded entry / remove from pending set and log the error });
apply the same pattern in sendCallInvite (set the invite tracking entry before
perspective.add() and remove it on .catch()) so
cleanupCallStartedLink()/cleanupInviteLinks() can reliably find and remove
persisted links regardless of timing.
---
Nitpick comments:
In `@app/src/components/call/controls/InviteToCallPopover.vue`:
- Around line 120-125: The inviteAll function currently sends invites but does
not clear selectedDids like sendInvites does; update inviteAll to reset
selectedDids (the same reactive/set used by sendInvites) after calling
webrtcStore.sendCallInvite(dids) and before removing the popover so both exit
paths behave identically — locate inviteAll, reference selectedDids and
sendInvites for the same reset behavior, and ensure
popover.value?.removeAttribute('open') remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 589be349-5e4d-4080-b089-7dcede95c8da
📒 Files selected for processing (3)
app/src/components/call/controls/InviteToCallPopover.vueapp/src/stores/webrtcStore.tsapp/src/utils/registerMobileNotifications.ts
| // If nobody else is in the call, persist a flux://call_started link so the | ||
| // AD4M notification trigger fires a push notification for the neighbourhood. | ||
| // Only the initiator emits this — late joiners don't, to avoid duplicate pushes. | ||
| // The link is cleaned up in leaveRoom() to prevent stale triggers. | ||
| if (agentsInCall.value.length === 0 && callRoute.value.channelId) { | ||
| const perspective = communityService.value?.perspective; | ||
| const channelUrl = restoreChannelPrefix(callRoute.value.channelId); | ||
| if (perspective) { | ||
| perspective | ||
| .add(new Link({ source: channelUrl, predicate: CALL_STARTED, target: channelUrl })) | ||
| .then(() => { | ||
| persistedCallStartedLink.value = { source: channelUrl, target: channelUrl }; | ||
| }) | ||
| .catch((error) => console.error('Failed to persist call_started link:', error)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Race between perspective.add(...).then(...) and leaveRoom() can leak persisted links.
The tracking refs are only populated in the .then() callback after perspective.add() resolves. If leaveRoom() runs between the link being persisted on the host and the .then() callback updating persistedCallStartedLink / persistedInviteLinks, cleanupCallStartedLink() / cleanupInviteLinks() will see null / empty array and skip removal, orphaning the link.
For CALL_STARTED this is especially undesirable — a leftover link can re-trigger flux://call_started push notifications on subsequent notification runtime evaluations. The same issue applies to sendCallInvite at lines 732-738.
Consider pre-recording the intended link before add() and clearing it in a .catch() on failure (or tracking a pending set), so cleanup is safe regardless of callback timing:
🛡️ Suggested pattern
if (perspective) {
- perspective
- .add(new Link({ source: channelUrl, predicate: CALL_STARTED, target: channelUrl }))
- .then(() => {
- persistedCallStartedLink.value = { source: channelUrl, target: channelUrl };
- })
- .catch((error) => console.error('Failed to persist call_started link:', error));
+ persistedCallStartedLink.value = { source: channelUrl, target: channelUrl };
+ perspective
+ .add(new Link({ source: channelUrl, predicate: CALL_STARTED, target: channelUrl }))
+ .catch((error) => {
+ persistedCallStartedLink.value = null;
+ console.error('Failed to persist call_started link:', error);
+ });
}Apply the same shape to sendCallInvite (push the tracking entry before add(), remove on .catch).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If nobody else is in the call, persist a flux://call_started link so the | |
| // AD4M notification trigger fires a push notification for the neighbourhood. | |
| // Only the initiator emits this — late joiners don't, to avoid duplicate pushes. | |
| // The link is cleaned up in leaveRoom() to prevent stale triggers. | |
| if (agentsInCall.value.length === 0 && callRoute.value.channelId) { | |
| const perspective = communityService.value?.perspective; | |
| const channelUrl = restoreChannelPrefix(callRoute.value.channelId); | |
| if (perspective) { | |
| perspective | |
| .add(new Link({ source: channelUrl, predicate: CALL_STARTED, target: channelUrl })) | |
| .then(() => { | |
| persistedCallStartedLink.value = { source: channelUrl, target: channelUrl }; | |
| }) | |
| .catch((error) => console.error('Failed to persist call_started link:', error)); | |
| } | |
| } | |
| // If nobody else is in the call, persist a flux://call_started link so the | |
| // AD4M notification trigger fires a push notification for the neighbourhood. | |
| // Only the initiator emits this — late joiners don't, to avoid duplicate pushes. | |
| // The link is cleaned up in leaveRoom() to prevent stale triggers. | |
| if (agentsInCall.value.length === 0 && callRoute.value.channelId) { | |
| const perspective = communityService.value?.perspective; | |
| const channelUrl = restoreChannelPrefix(callRoute.value.channelId); | |
| if (perspective) { | |
| persistedCallStartedLink.value = { source: channelUrl, target: channelUrl }; | |
| perspective | |
| .add(new Link({ source: channelUrl, predicate: CALL_STARTED, target: channelUrl })) | |
| .catch((error) => { | |
| persistedCallStartedLink.value = null; | |
| console.error('Failed to persist call_started link:', error); | |
| }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/stores/webrtcStore.ts` around lines 633 - 648, The persisted link can
be orphaned because persistedCallStartedLink (and persistedInviteLinks) are only
set in the perspective.add(...).then(...) callback, so if
leaveRoom()/cleanupCallStartedLink() runs before the promise resolves the
cleanup skips removal; to fix, pre-record the intended link entry before calling
perspective.add (i.e. set persistedCallStartedLink = { ... } or push into
persistedInviteLinks immediately), then call perspective.add(...).catch(error =>
{ clear the pre-recorded entry / remove from pending set and log the error });
apply the same pattern in sendCallInvite (set the invite tracking entry before
perspective.add() and remove it on .catch()) so
cleanupCallStartedLink()/cleanupInviteLinks() can reliably find and remove
persisted links regardless of timing.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/src/stores/webrtcStore.ts (1)
644-649:⚠️ Potential issue | 🟡 MinorTrack persisted links before awaiting
add().The tracking refs are still populated only in
.then(), soleaveRoom()can run before cleanup knows about the pending link. Pre-record the intended link beforeperspective.add(...), then clear that entry in.catch(...)if the add fails.Also applies to: 743-748
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/stores/webrtcStore.ts` around lines 644 - 649, Pre-populate the tracking ref persistedCallStartedLink with the intended link (e.g., { source: channelUrl, target: channelUrl }) before calling perspective.add(...) so leaveRoom() can see the pending link; then call perspective.add(...). In the promise handlers, clear persistedCallStartedLink in .catch(...) if add fails and keep or confirm the value in .then(...); apply the same change for the other occurrence referenced around lines 743-748 to ensure both pending links are recorded before the async add call and cleaned on error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/stores/webrtcStore.ts`:
- Around line 640-643: The DIAG console.log calls leak sensitive identifiers
(recipient DIDs, channel/perspective IDs); find and remove or redact any
console.log lines containing "[DIAG" (examples around the Link/CALL_STARTED
creation and the uses of communityService, perspective, channelUrl, and Link)
and either delete them or replace them with the existing debug/dev logger (only
when enabled) and log non-sensitive info (e.g., booleans or masked IDs like
"<redacted>"). Ensure you update all occurrences (the shown snippet and the
other DIAG logs near the Link/CALL_STARTED uses) so no raw
DIDs/channel/perspective identifiers are printed.
- Around line 670-672: The cleanup functions cleanupInviteLinks and
cleanupCallStartedLink currently clear local tracking immediately after calling
perspective.remove(...), so if remove fails the persisted link remains but local
state was dropped; modify these functions to be async and await the
perspective.remove call (or await a Promise.all if removing multiple links),
only clear the local tracking state after the remove promise resolves
successfully, and on error log (processLogger/errorLogger) and keep the tracking
state so the cleanup can be retried; apply the same change to the analogous
cleanup block at the other location mentioned (around the other 761-781
section).
---
Duplicate comments:
In `@app/src/stores/webrtcStore.ts`:
- Around line 644-649: Pre-populate the tracking ref persistedCallStartedLink
with the intended link (e.g., { source: channelUrl, target: channelUrl }) before
calling perspective.add(...) so leaveRoom() can see the pending link; then call
perspective.add(...). In the promise handlers, clear persistedCallStartedLink in
.catch(...) if add fails and keep or confirm the value in .then(...); apply the
same change for the other occurrence referenced around lines 743-748 to ensure
both pending links are recorded before the async add call and cleaned on error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4a86952e-c245-45cd-91b9-26285bdce54b
📒 Files selected for processing (1)
app/src/stores/webrtcStore.ts
| console.log('[DIAG call_started] communityService:', !!communityService.value, 'perspective:', !!perspective, 'uuid:', perspective?.uuid, 'channelUrl:', channelUrl); | ||
| if (perspective) { | ||
| const link = new Link({ source: channelUrl, predicate: CALL_STARTED, target: channelUrl }); | ||
| console.log('[DIAG call_started] adding link:', JSON.stringify(link)); |
There was a problem hiding this comment.
Remove or redact diagnostic call-notification logs before merge.
These logs expose invite recipient DIDs plus channel/community/perspective identifiers. Keep only redacted debug output behind an existing dev/debug logger, or remove the [DIAG ...] logging entirely.
Also applies to: 729-729, 741-742, 750-752
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/stores/webrtcStore.ts` around lines 640 - 643, The DIAG console.log
calls leak sensitive identifiers (recipient DIDs, channel/perspective IDs); find
and remove or redact any console.log lines containing "[DIAG" (examples around
the Link/CALL_STARTED creation and the uses of communityService, perspective,
channelUrl, and Link) and either delete them or replace them with the existing
debug/dev logger (only when enabled) and log non-sensitive info (e.g., booleans
or masked IDs like "<redacted>"). Ensure you update all occurrences (the shown
snippet and the other DIAG logs near the Link/CALL_STARTED uses) so no raw
DIDs/channel/perspective identifiers are printed.
| // Clean up persisted call invite links to prevent stale notifications | ||
| cleanupInviteLinks(); | ||
| cleanupCallStartedLink(); |
There was a problem hiding this comment.
Await removals before dropping cleanup state.
cleanupInviteLinks() / cleanupCallStartedLink() clear local tracking immediately after starting perspective.remove(...). If removal fails, the link remains persisted but can no longer be retried, causing stale notifications.
Suggested cleanup shape
- cleanupInviteLinks();
- cleanupCallStartedLink();
+ await cleanupInviteLinks();
+ await cleanupCallStartedLink();- function cleanupInviteLinks(): void {
+ async function cleanupInviteLinks(): Promise<void> {
const perspective = communityService.value?.perspective;
if (!perspective || !persistedInviteLinks.value.length) return;
- for (const invite of persistedInviteLinks.value) {
- perspective
- .remove(new Link({ source: invite.source, predicate: CALL_INVITE, target: invite.target }))
- .catch((error) => console.error('Failed to remove call invite link:', error));
- }
- persistedInviteLinks.value = [];
+ const invites = [...persistedInviteLinks.value];
+ const failed: Array<{ source: string; target: string }> = [];
+
+ await Promise.all(
+ invites.map(async (invite) => {
+ try {
+ await perspective.remove(new Link({ source: invite.source, predicate: CALL_INVITE, target: invite.target }));
+ } catch (error) {
+ failed.push(invite);
+ console.error('Failed to remove call invite link:', error);
+ }
+ }),
+ );
+
+ persistedInviteLinks.value = failed;
}
- function cleanupCallStartedLink(): void {
+ async function cleanupCallStartedLink(): Promise<void> {
const perspective = communityService.value?.perspective;
const link = persistedCallStartedLink.value;
if (!perspective || !link) return;
- perspective
- .remove(new Link({ source: link.source, predicate: CALL_STARTED, target: link.target }))
- .catch((error) => console.error('Failed to remove call_started link:', error));
- persistedCallStartedLink.value = null;
+ try {
+ await perspective.remove(new Link({ source: link.source, predicate: CALL_STARTED, target: link.target }));
+ persistedCallStartedLink.value = null;
+ } catch (error) {
+ console.error('Failed to remove call_started link:', error);
+ }
}Also applies to: 761-781
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/stores/webrtcStore.ts` around lines 670 - 672, The cleanup functions
cleanupInviteLinks and cleanupCallStartedLink currently clear local tracking
immediately after calling perspective.remove(...), so if remove fails the
persisted link remains but local state was dropped; modify these functions to be
async and await the perspective.remove call (or await a Promise.all if removing
multiple links), only clear the local tracking state after the remove promise
resolves successfully, and on error log (processLogger/errorLogger) and keep the
tracking state so the cleanup can be retried; apply the same change to the
analogous cleanup block at the other location mentioned (around the other
761-781 section).
2 participants
Summary
http://push-notifications.ad4m.dev:13000/notificationtohttps://push.ad4m.dev/notificationensureNotification()helper to avoid duplicationcreateCommunityandjoinCommunityto add new perspective UUIDs to all Flux notifications (mentions + calls), not just the first one foundDetails
The call notification trigger uses a SPARQL query matching
agent/new-statebroadcast links whereinCallistrue, filtering out the user's own DID to avoid self-notifications.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes