Skip to content

fix(desktop): dedupe Nostr session deep link imports#9918

Open
harrykamboj1 wants to merge 4 commits into
aaif-goose:mainfrom
harrykamboj1:fix/nostr-deeplink-duplicate-import
Open

fix(desktop): dedupe Nostr session deep link imports#9918
harrykamboj1 wants to merge 4 commits into
aaif-goose:mainfrom
harrykamboj1:fix/nostr-deeplink-duplicate-import

Conversation

@harrykamboj1

Copy link
Copy Markdown

Summary

Fix duplicate Nostr session imports from goose://sessions/nostr deep links.
macOS could deliver multiple open-url events for one Chrome paste, triggering
several POST /sessions/import/nostr calls.

Changes:

  • main.ts: queue links while window loading, 2s burst dedup, align macOS with Windows behavior
  • App.tsx: in-flight + post-success dedup for Nostr deep link imports only

Testing

Manual testing on macOS:

  • Cold start via Chrome deep link → 1 session
  • App already running → 1 session
  • Repeated IPC simulation in DevTools → 1 API call
  • Manual Import Link in Session History → unchanged

Related Issues

Fixes #9917

Screenshots

Before: multiple identical sessions in Chat History from one link
Screenshot 2026-06-21 at 1 51 40 PM

After: single session imported
Screenshot 2026-06-21 at 1 56 50 PM

Signed-off-by: harrykamboj1 <singhharnoor116@gmail.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f0e62bc81

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/desktop/src/main.ts Outdated
Comment on lines +526 to +528
if (!pendingDeepLinks.has(targetWindow.id)) {
pendingDeepLinks.set(targetWindow.id, url);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid dropping newer pending deep links

When the target window is still loading, this has guard keeps the first pending URL and silently ignores every later extension/session deep link for the same window, even when the URL is different. During startup or a renderer reload, opening an extension link and then a Nostr/session link before react-ready means only the stale first URL is delivered at react-ready; the newer user action is lost. The dedupe should only collapse identical URLs or queue/replace distinct pending links.

Useful? React with 👍 / 👎.

@DOsinga

DOsinga commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the fix and the detailed issue (#9917) — nice to see a small, well-scoped change with repro and before/after screenshots.

A couple of things before this can land:

  1. Codex's P2 is valid and needs addressing. The new pendingDeepLinks.has() guard (in deliverExtensionOrSessionDeepLink, handleProtocolUrl, and the open-url new-window path) collapses distinct pending links, not just identical bursts. During startup/renderer reload, if an extension link is queued and then a sessions/Nostr link arrives before react-ready, the newer user action is silently dropped and only the stale first URL is delivered. The dedupe should only collapse identical URLs — distinct pending links should queue or replace.

  2. The renderer-side dedup overlaps the main-process burst dedup — please simplify. You now have three time windows in play: the 2s main-process burst (DEEPLINK_BURST_DEDUP_MS), the 5s renderer post-success map (NOSTR_IMPORT_SUCCESS_DEDUP_MS), plus the implicit in-flight import duration. The nostrImportInFlight in-flight guard earns its keep — it protects the skipBurstDedup: true pending-link path and the case where macOS fires events spaced further apart than the 2s window while an import is still running. But the 5s recentSuccessfulNostrImports map largely duplicates what the 2s main-process burst window already does. Can you either drop the post-success map and rely on in-flight + main burst dedup, or explain why both windows (and the differing 2s/5s values) are needed? Two independent magic windows for the same race is hard to reason about and maintain.

I'll snooze this for a few days to give you a chance to respond.

@harrykamboj1

Copy link
Copy Markdown
Author

Hi @DOsinga
Thanks for the review. I've addressed both points in the latest push:

  1. P2 / pendingDeepLinks: Replaced the has() guards with queuePendingDeepLink(), which only skips identical URLs and replaces the pending entry when a different deep link arrives before react-ready (latest user action wins during startup).

  2. Renderer dedup: Removed the 5s recentSuccessfulNostrImports map. Kept nostrImportInFlight for concurrent imports and the 2s main-process burst dedup in sendOpenSharedSession().

Let me know if you'd like anything else adjusted.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a5ec09466

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread ui/desktop/src/main.ts
}

const DEEPLINK_BURST_DEDUP_MS = 2000;
let lastSentSessionDeepLink: { url: string; at: number } | null = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Track all recent session links during burst dedup

With two different session/Nostr links opened during the same 2s macOS duplicate burst, this single-slot cache only suppresses adjacent duplicates. For an event order A, B, A, sending B overwrites A's timestamp, so the later duplicate A is sent to the renderer again and can still create a duplicate Nostr import. Keep recent send times keyed by URL until the burst window expires instead of storing only the last URL.

Useful? React with 👍 / 👎.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Opening goose://sessions/nostr deep link creates multiple duplicate session imports (macOS)

2 participants