Skip to content

Fix relay reconnect notification loop#47

Merged
TraderSamwise merged 2 commits into
masterfrom
fix/relay-reconnect-notification-loop
May 30, 2026
Merged

Fix relay reconnect notification loop#47
TraderSamwise merged 2 commits into
masterfrom
fix/relay-reconnect-notification-loop

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented May 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • keep the app relay transport stable across auth/render updates
  • pause relay-backed polling until the daemon is connected through the relay
  • dedupe daemon native remote-client-connected notifications for bouncing clients

Verification

  • yarn typecheck
  • yarn lint
  • yarn test src/relay-client.test.ts
  • yarn --cwd app typecheck
  • yarn --cwd app test lib/relay-transport.test.ts
  • yarn --cwd app lint
  • pre-push hook: yarn typecheck && yarn lint && yarn test

Summary by CodeRabbit

  • Bug Fixes

    • Prevent duplicate remote-client connection notifications by deduplicating repeated events.
    • Stop data polling (projects, desktop state, notifications) until the connection relay is ready to avoid premature updates.
    • Improve authentication token handling to ensure reliable authenticated connections.
  • Tests

    • Added tests verifying dedupe behavior and adjusted timers/cleanup for notification-related tests.

Review Change Stack

@vercel

vercel Bot commented May 30, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment May 30, 2026 3:32am

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a48659bc-d159-4285-91f9-d75ae66d8e10

📥 Commits

Reviewing files that changed from the base of the PR and between ff25abc and a9678ce.

📒 Files selected for processing (1)
  • app/app/(main)/_layout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/app/(main)/_layout.tsx

📝 Walkthrough

Walkthrough

MainLayout now computes relayUrl and relayReadyForRequests, uses a getTokenRef to supply the latest token callback to relay transport and push-token registration, and gates non-shared projects/desktop/notification polling until the relay is ready. RelayClient adds deduplication for repeated client_connected notifications keyed by deviceId (or title:body) using a time window, with tests verifying behavior.

Changes

Relay readiness and remote client deduplication

Layer / File(s) Summary
Relay readiness infrastructure and token ref
app/app/(main)/_layout.tsx
Centralizes relay readiness by computing relayUrl and relayReadyForRequests, derives active-share identifiers/options, and sets up getTokenRef with useRef + effect so token callbacks always read the latest auth function.
Relay transport setup with readiness-aware token callback
app/app/(main)/_layout.tsx
Reworks the relay transport effect to construct RelayTransport with a token callback backed by getTokenRef; skips transport setup when relayUrl is unset; passes derived active-share relay options and removes getToken/full activeShare from effect dependencies.
Polling effects gated by relay readiness
app/app/(main)/_layout.tsx
Projects, desktop-state, and notifications polling effects short-circuit (early-return) when relayReadyForRequests is false and include relayReadyForRequests in their dependency arrays so polling resumes once ready.
Remote client connection notification deduplication
src/relay-client.ts
Adds REMOTE_CLIENT_NOTIFICATION_DEDUPE_MS and a recentRemoteClientNotifications map; computes a dedupe key from deviceId (or title:body) and enforces a dedupe window via shouldNotifyRemoteClientConnected before calling notifyRemoteClientConnected.
Deduplication test setup and verification
src/relay-client.test.ts
Adds beforeEach to clear notification mocks and afterEach to restore real timers; extends the client_connected fixture with deviceId; adds a test using fake timers that verifies repeated client_connected events are deduplicated and that notifications reappear after the dedupe interval elapses.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • TraderSamwise/aimux#20: Main PR updates MainLayout to derive and pass ownerUserId/shareId relay options (and gate relay-dependent polling) in a way that directly matches the shared-relay client routing changes introduced in PR #20's RelayTransport/owner-share connection model.

Poem

🐰 A relay waits on patient paws tonight,
Tokens tucked in a steady, ref-lit light,
Polling sleeps until the cable sings,
Bouncing clients hush — dedupe springs,
Tests hop by to make the quiet right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title directly addresses the main change: a notification deduplication mechanism to fix repeated relay reconnect notifications. The title accurately reflects the primary problem being solved.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/relay-reconnect-notification-loop

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

app/app/(main)/_layout.tsx

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/app/(main)/_layout.tsx (1)

132-140: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t block local shared-session reconciliation on relay readiness.

Line 134 returns before the activeShare branch runs, so a shared session no longer updates projectsAtom / selection state while the relay is reconnecting. That branch is local state sync, not relay-backed polling, so this widens the pause beyond the PR goal.

Suggested fix
     async function loop() {
       if (cancelled) return;
-      if (!relayReadyForRequests) return;
       if (activeShare) {
         reconcileProjects([projectFromActiveShare(activeShare)]);
         store.set(selectedProjectPathAtom, activeShare.projectRoot);
         store.set(selectedSessionIdAtom, activeShare.sessionId);
         timer = setTimeout(loop, POLL_INTERVAL_MS);
         return;
       }
+      if (!relayReadyForRequests) return;
       try {
         const token = await getToken();
         const projects = await listProjects({ token });
🤖 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 `@app/app/`(main)/_layout.tsx around lines 132 - 140, The loop function
currently returns early when !relayReadyForRequests which prevents the
activeShare handling from running; move or split the checks so that activeShare
reconciliation always runs regardless of relayReadyForRequests. Specifically, in
the loop function (symbols: loop, relayReadyForRequests, activeShare,
reconcileProjects, projectFromActiveShare, store.set(selectedProjectPathAtom),
store.set(selectedSessionIdAtom), timer, POLL_INTERVAL_MS) ensure the
activeShare branch executes before or independent of the relay readiness check
and still schedules the next timer; only the relay-backed polling logic should
be skipped when relayReadyForRequests is false.
🤖 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.

Outside diff comments:
In `@app/app/`(main)/_layout.tsx:
- Around line 132-140: The loop function currently returns early when
!relayReadyForRequests which prevents the activeShare handling from running;
move or split the checks so that activeShare reconciliation always runs
regardless of relayReadyForRequests. Specifically, in the loop function
(symbols: loop, relayReadyForRequests, activeShare, reconcileProjects,
projectFromActiveShare, store.set(selectedProjectPathAtom),
store.set(selectedSessionIdAtom), timer, POLL_INTERVAL_MS) ensure the
activeShare branch executes before or independent of the relay readiness check
and still schedules the next timer; only the relay-backed polling logic should
be skipped when relayReadyForRequests is false.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: dadadd9f-f4e5-48fe-b47c-d4b9a1d9dccb

📥 Commits

Reviewing files that changed from the base of the PR and between 1d36ed6 and ff25abc.

📒 Files selected for processing (3)
  • app/app/(main)/_layout.tsx
  • src/relay-client.test.ts
  • src/relay-client.ts

@TraderSamwise

Copy link
Copy Markdown
Owner Author

Fixed in a9678ce - shared-session reconciliation now runs before the relay-readiness gate, so only relay-backed polling pauses during reconnect.

@TraderSamwise
TraderSamwise merged commit 85442a1 into master May 30, 2026
3 checks passed
@TraderSamwise
TraderSamwise deleted the fix/relay-reconnect-notification-loop branch May 30, 2026 03:34
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.

1 participant