drop declined social bindings via source-of-truth reconcile#1068
drop declined social bindings via source-of-truth reconcile#1068Bekiboo wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
👮 Files not reviewed due to content moderation or server errors (1)
📝 Walkthrough🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/eid-wallet/src/routes/(app)/main/+page.svelte (1)
423-436: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winThrottle social-binding reconciliation on passive refreshes.
refreshBindings()callsloadBindingDocuments()on mount, every 30s, and on visibility changes, sofetchReconciledSocialBindings()now fans out to remote vault lookups repeatedly for each unconfirmedsentbinding. Cache or skip recent reconciliation runs so foreground polling doesn’t turn into repeated cross-vault traffic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte around lines 423 - 436, Throttle the social-binding reconciliation in loadSocialBindings so refreshBindings does not repeatedly trigger cross-vault fetches on mount, interval polling, and visibility changes. Add a recent-run cache or debounce/skip guard around fetchReconciledSocialBindings in loadSocialBindings/loadBindingDocuments so unconfirmed sent bindings are not reconciled multiple times in quick succession. Use the existing loadSocialBindings and refreshBindings entry points to centralize the cooldown logic and prevent repeated remote vault traffic.
🧹 Nitpick comments (3)
infrastructure/eid-wallet/src/lib/utils/socialBinding.ts (1)
707-718: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDeduplicate remote reads per counterparty.
summaries.map(...)fires onefetchSentBindingStatuspersentmirror, and each call does aresolveVaultUrifetch plus a GraphQL request. When the same counterparty appears in multiplesentdocs (the grouping in both consuming pages assumes this happens), you repeatedly resolve and query the identical vault. Consider caching the status bycounterpartyEnamefor the duration of one reconcile.♻️ Sketch: memoize status per counterparty
const summaries = await fetchSocialBindings(ownGqlUrl, callerEname); +const statusCache = new Map<string, Promise<SentBindingStatus>>(); +const getStatus = (counterparty: string) => { + let p = statusCache.get(counterparty); + if (!p) { + p = fetchSentBindingStatus(callerEname, counterparty); + statusCache.set(counterparty, p); + } + return p; +}; const reconciled = await Promise.all( summaries.map(async (summary) => { if (summary.role !== "sent" || summary.mutuallySigned) { return summary; } try { - const status = await fetchSentBindingStatus( - callerEname, - summary.counterpartyEname, - ); + const status = await getStatus(summary.counterpartyEname);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/lib/utils/socialBinding.ts` around lines 707 - 718, The reconcile flow in socialBinding.ts is redundantly calling fetchSentBindingStatus for multiple sent summaries with the same counterpartyEname, causing repeated resolveVaultUri and GraphQL reads. Update the Promise.all mapping in the reconcile logic to memoize the remote status per counterpartyEname for the duration of one run, so repeated sent mirrors reuse the same in-flight or cached result instead of re-querying. Keep the fix local to the reconcile path and preserve the existing behavior for non-sent or mutuallySigned summaries.infrastructure/eid-wallet/src/routes/(app)/main/+page.svelte (1)
456-490: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPending-group computation duplicated across pages.
The
hasSent/hasReceived/pendinggrouping logic here mirrors what the stack outline describes forsocial-bindings/+page.svelte(same cohort, "computes pending per group"). Duplicating this logic in two Svelte pages risks the two views drifting out of sync if reconciliation semantics change later.Consider extracting a shared helper (e.g.
groupSocialBindingsByContact/buildSocialBindingDisplay) into$lib/utils/socialBinding.tsand reusing it from bothmain/+page.svelteandsocial-bindings/+page.svelte.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte around lines 456 - 490, The pending-group and role calculation is duplicated in this page and in the social-bindings page, so extract the shared grouping/display logic into a reusable helper such as groupSocialBindingsByContact or buildSocialBindingDisplay in $lib/utils/socialBinding.ts. Move the hasSent, hasReceived, role, and pending computation out of the inline groupedSlice.map callback in main/+page.svelte and reuse the same helper from social-bindings/+page.svelte to keep both views consistent.infrastructure/eid-wallet/src/routes/(app)/social-bindings/+page.svelte (1)
68-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the group-aggregation logic to a shared helper.
The role-derivation/
pending-computation/name-resolution logic that buildsSocialBindingDisplayhere appears to be duplicated in main/+page.svelte within the same PR stack (per the stack outline, that page also "computes a pending flag per counterparty group"). Since both pages need identical grouping semantics againstSocialBindingSummary, extracting this into a shared utility (e.g. alongsidefetchReconciledSocialBindingsinsocialBinding.ts) would prevent the two implementations from silently diverging over time.The
pending/rolecomputation itself is correct here and matches the documentedSocialBindingDisplaycontract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/eid-wallet/src/routes/`(app)/social-bindings/+page.svelte around lines 68 - 104, The `SocialBindingDisplay` group-building logic in the `rows` aggregation is duplicated with the matching counterparty-group handling in main/+page.svelte and should be centralized. Extract the shared role/pending/name-resolution behavior into a reusable helper near `fetchReconciledSocialBindings` in `socialBinding.ts`, then have both pages call that helper so the `role`, `pending`, and name lookup semantics stay consistent and don’t diverge.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infrastructure/eid-wallet/src/lib/utils/socialBinding.ts`:
- Around line 670-686: The reconciliation logic in fetchReconciledSocialBindings
is treating an empty first page from bindingDocuments as a real “declined”
state. Update the social connection lookup in socialBinding.ts to page through
all bindingDocuments results instead of relying on the hard first: 50 cap, and
only return "declined" after all pages have been checked and no matching
BindingDocParsed entry is found.
---
Outside diff comments:
In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte:
- Around line 423-436: Throttle the social-binding reconciliation in
loadSocialBindings so refreshBindings does not repeatedly trigger cross-vault
fetches on mount, interval polling, and visibility changes. Add a recent-run
cache or debounce/skip guard around fetchReconciledSocialBindings in
loadSocialBindings/loadBindingDocuments so unconfirmed sent bindings are not
reconciled multiple times in quick succession. Use the existing
loadSocialBindings and refreshBindings entry points to centralize the cooldown
logic and prevent repeated remote vault traffic.
---
Nitpick comments:
In `@infrastructure/eid-wallet/src/lib/utils/socialBinding.ts`:
- Around line 707-718: The reconcile flow in socialBinding.ts is redundantly
calling fetchSentBindingStatus for multiple sent summaries with the same
counterpartyEname, causing repeated resolveVaultUri and GraphQL reads. Update
the Promise.all mapping in the reconcile logic to memoize the remote status per
counterpartyEname for the duration of one run, so repeated sent mirrors reuse
the same in-flight or cached result instead of re-querying. Keep the fix local
to the reconcile path and preserve the existing behavior for non-sent or
mutuallySigned summaries.
In `@infrastructure/eid-wallet/src/routes/`(app)/main/+page.svelte:
- Around line 456-490: The pending-group and role calculation is duplicated in
this page and in the social-bindings page, so extract the shared
grouping/display logic into a reusable helper such as
groupSocialBindingsByContact or buildSocialBindingDisplay in
$lib/utils/socialBinding.ts. Move the hasSent, hasReceived, role, and pending
computation out of the inline groupedSlice.map callback in main/+page.svelte and
reuse the same helper from social-bindings/+page.svelte to keep both views
consistent.
In `@infrastructure/eid-wallet/src/routes/`(app)/social-bindings/+page.svelte:
- Around line 68-104: The `SocialBindingDisplay` group-building logic in the
`rows` aggregation is duplicated with the matching counterparty-group handling
in main/+page.svelte and should be centralized. Extract the shared
role/pending/name-resolution behavior into a reusable helper near
`fetchReconciledSocialBindings` in `socialBinding.ts`, then have both pages call
that helper so the `role`, `pending`, and name lookup semantics stay consistent
and don’t diverge.
🪄 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: ba62ab24-2725-45d0-8f5a-f6c04d5fb1e7
📒 Files selected for processing (6)
infrastructure/eid-wallet/src/lib/utils/socialBinding.tsinfrastructure/eid-wallet/src/routes/(app)/main/+page.svelteinfrastructure/eid-wallet/src/routes/(app)/main/components/SocialBindingAccordion.svelteinfrastructure/eid-wallet/src/routes/(app)/main/components/SocialBindingDetailsSheet.svelteinfrastructure/eid-wallet/src/routes/(app)/scan-qr/components/SocialBindingDrawer.svelteinfrastructure/eid-wallet/src/routes/(app)/social-bindings/+page.svelte
1 participant
Description of change
Social bindings kept showing as successful on the scanner's device even after the requester declined (#990). The scanner writes a single-signature mirror into its own vault on acceptance; the requester's decline only deletes the primary doc from its own vault (vault isolation blocks cross-vault deletion) and the scanner never re-checked, so its mirror lingered forever. This reconciles scanner-initiated (
sent) mirrors against the counterparty's vault (the source of truth) on every list load: counter-signed → confirmed, gone → dropped + orphan mirror deleted, still pending/offline → kept as Awaiting confirmation.Issue Number
Closes #990
Type of change
How the change has been tested
pnpm --filter eid-wallet check→ svelte-check 0 errors, Biome clean.@…→ vault URI) + GraphQL read ofsocial_connectiondocs returned HTTP 200 with no auth/scoping error, confirming the reconcile actually executes rather than silently no-op'ing.findBindingDocuments→ the same paginated DB method exercised by themetaEnvelopespagination spec); a live paginated round-trip was blocked by an unrelated local-stack outage (allbindingDocumentsqueries 500'd, including a previously-passing one).Change checklist
Design decisions
declined; an unreachable vault keeps it pending.declinedis concluded only after every page has been checked (viaafter/pageInfocursor pagination), so a doc merely absent from the first page can't trigger a wrongful mirror deletion.A detailed high-level summary could not be generated for this review. Here is an overview derived from the analyzed file changes: