Skip to content

Commit 1474185

Browse files
committed
fix(collab): preserve presence through lease gaps
1 parent fa9a38c commit 1474185

2 files changed

Lines changed: 231 additions & 26 deletions

File tree

apps/web/src/collab/collab-client.ts

Lines changed: 76 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ export interface CollabClientOptions {
7373

7474
const DEFAULT_HEARTBEAT_MS = 10_000;
7575
const DEFAULT_STATUS_POLL_MS = 5_000;
76+
// Vela's authoritative project-presence lease is 30 seconds from the
77+
// server-recorded heartbeatAt, not from when this client happens to observe
78+
// the roster. Using observation time can nearly double a stale lease.
79+
const DEFAULT_PRESENCE_ROSTER_GRACE_MS = 30_000;
7680

7781
class CollabRequestError extends Error {
7882
constructor(message: string, readonly status: number) {
@@ -132,8 +136,10 @@ export class CollabClient {
132136
*/
133137
private presenceRequestGeneration = 0;
134138
private presenceAppliedRequestGeneration = 0;
135-
/** First successful self-bearing response that omitted each known peer. */
136-
private readonly presenceMissingSince = new Map<string, number>();
139+
/** Authoritative Vela lease expiry derived from each valid heartbeatAt. */
140+
private readonly presenceLeaseExpiresAt = new Map<string, number>();
141+
/** Compatibility grace for legacy/local rosters without a valid heartbeatAt. */
142+
private readonly presenceFallbackMissingSince = new Map<string, number>();
137143
/** Suppresses the status-transition echo of an optimistic Team heartbeat. */
138144
private presenceAttemptedForMember = false;
139145
/**
@@ -252,7 +258,7 @@ export class CollabClient {
252258
// the roster converged. This fresh read is only an extra catch-up for
253259
// events/timers the browser may have throttled in the background, and
254260
// it preserves the current roster until the response lands.
255-
void this.refreshPresence();
261+
void this.refreshPresenceAfterVisibility();
256262
void this.pollStatus();
257263
};
258264
document.addEventListener('visibilitychange', this.onVisibilityChange);
@@ -265,7 +271,8 @@ export class CollabClient {
265271
this.lifecycleGeneration += 1;
266272
this.presenceRequestGeneration += 1;
267273
this.presenceAppliedRequestGeneration = this.presenceRequestGeneration;
268-
this.presenceMissingSince.clear();
274+
this.presenceLeaseExpiresAt.clear();
275+
this.presenceFallbackMissingSince.clear();
269276
for (const timer of this.timers) clearInterval(timer);
270277
this.timers.length = 0;
271278
if (this.onVisibilityChange && typeof document !== 'undefined') {
@@ -353,6 +360,18 @@ export class CollabClient {
353360
* feedback loop, so push-channel consumers must use this read-only path.
354361
*/
355362
async refreshPresence(): Promise<void> {
363+
await this.readFreshPresence(false);
364+
}
365+
366+
/**
367+
* Visibility catch-up is not evidence of an explicit viewer-set mutation.
368+
* Preserve still-live last-good peers just like a heartbeat response.
369+
*/
370+
private async refreshPresenceAfterVisibility(): Promise<void> {
371+
await this.readFreshPresence(true);
372+
}
373+
374+
private async readFreshPresence(preserveMissingPeers: boolean): Promise<void> {
356375
const requestGeneration = ++this.presenceRequestGeneration;
357376
try {
358377
// A hub presence event marks the daemon's short-lived roster cache stale.
@@ -365,6 +384,7 @@ export class CollabClient {
365384
this.applyPresenceResponse(
366385
requestGeneration,
367386
body.present as CollabPresenceMember[],
387+
preserveMissingPeers,
368388
);
369389
}
370390
} catch (error) {
@@ -562,10 +582,13 @@ export class CollabClient {
562582
private applyPresenceResponse(
563583
requestGeneration: number,
564584
present: CollabPresenceMember[],
585+
preserveMissingPeers = true,
565586
): void {
566587
if (requestGeneration <= this.presenceAppliedRequestGeneration) return;
567588
this.presenceAppliedRequestGeneration = requestGeneration;
568-
this.update({ present: this.stabilizePresenceRoster(present) });
589+
this.update({
590+
present: this.stabilizePresenceRoster(present, preserveMissingPeers),
591+
});
569592
}
570593

571594
private applyPresenceAuthorityRevocation(requestGeneration: number): void {
@@ -575,12 +598,14 @@ export class CollabClient {
575598
}
576599

577600
private clearPresenceRoster(): void {
578-
this.presenceMissingSince.clear();
601+
this.presenceLeaseExpiresAt.clear();
602+
this.presenceFallbackMissingSince.clear();
579603
if (this.snapshot.present.length > 0) this.update({ present: [] });
580604
}
581605

582606
private stabilizePresenceRoster(
583607
incoming: CollabPresenceMember[],
608+
preserveMissingPeers: boolean,
584609
): CollabPresenceMember[] {
585610
const selfMemberId = this.member?.memberId;
586611
const incomingIds = new Set(incoming.map((member) => member.memberId));
@@ -592,13 +617,35 @@ export class CollabClient {
592617
|| incoming.length === 0
593618
|| !incomingIds.has(selfMemberId)
594619
) {
595-
this.presenceMissingSince.clear();
620+
this.presenceLeaseExpiresAt.clear();
621+
this.presenceFallbackMissingSince.clear();
596622
return incoming;
597623
}
598624

599625
const now = Date.now();
600-
for (const memberId of incomingIds) {
601-
this.presenceMissingSince.delete(memberId);
626+
for (const member of incoming) {
627+
const heartbeatAt = member.heartbeatAt?.trim();
628+
const heartbeatAtMs = heartbeatAt ? Date.parse(heartbeatAt) : Number.NaN;
629+
if (Number.isFinite(heartbeatAtMs)) {
630+
this.presenceLeaseExpiresAt.set(
631+
member.memberId,
632+
heartbeatAtMs + DEFAULT_PRESENCE_ROSTER_GRACE_MS,
633+
);
634+
} else {
635+
this.presenceLeaseExpiresAt.delete(member.memberId);
636+
}
637+
this.presenceFallbackMissingSince.delete(member.memberId);
638+
}
639+
if (!preserveMissingPeers) {
640+
for (const memberId of this.presenceLeaseExpiresAt.keys()) {
641+
if (!incomingIds.has(memberId)) this.presenceLeaseExpiresAt.delete(memberId);
642+
}
643+
for (const memberId of this.presenceFallbackMissingSince.keys()) {
644+
if (!incomingIds.has(memberId)) {
645+
this.presenceFallbackMissingSince.delete(memberId);
646+
}
647+
}
648+
return incoming;
602649
}
603650

604651
const retained: CollabPresenceMember[] = [];
@@ -607,18 +654,33 @@ export class CollabClient {
607654
);
608655
for (const member of this.snapshot.present) {
609656
if (incomingIds.has(member.memberId)) continue;
610-
const missingSince = this.presenceMissingSince.get(member.memberId);
657+
const leaseExpiresAt = this.presenceLeaseExpiresAt.get(member.memberId);
658+
if (leaseExpiresAt !== undefined) {
659+
if (now < leaseExpiresAt) retained.push(member);
660+
else this.presenceLeaseExpiresAt.delete(member.memberId);
661+
this.presenceFallbackMissingSince.delete(member.memberId);
662+
continue;
663+
}
664+
665+
const missingSince = this.presenceFallbackMissingSince.get(member.memberId);
611666
if (missingSince === undefined) {
612-
this.presenceMissingSince.set(member.memberId, now);
667+
this.presenceFallbackMissingSince.set(member.memberId, now);
613668
retained.push(member);
614669
} else if (now - missingSince < this.heartbeatMs) {
615670
retained.push(member);
616671
} else {
617-
this.presenceMissingSince.delete(member.memberId);
672+
this.presenceFallbackMissingSince.delete(member.memberId);
618673
}
619674
}
620-
for (const memberId of this.presenceMissingSince.keys()) {
621-
if (!previousIds.has(memberId)) this.presenceMissingSince.delete(memberId);
675+
for (const memberId of this.presenceLeaseExpiresAt.keys()) {
676+
if (!previousIds.has(memberId) && !incomingIds.has(memberId)) {
677+
this.presenceLeaseExpiresAt.delete(memberId);
678+
}
679+
}
680+
for (const memberId of this.presenceFallbackMissingSince.keys()) {
681+
if (!previousIds.has(memberId) && !incomingIds.has(memberId)) {
682+
this.presenceFallbackMissingSince.delete(memberId);
683+
}
622684
}
623685
return retained.length > 0 ? [...incoming, ...retained] : incoming;
624686
}

apps/web/tests/collab-client.test.ts

Lines changed: 155 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
2-
import { CollabClient, type CollabSnapshot } from '../src/collab/collab-client.js';
2+
import {
3+
CollabClient,
4+
type CollabPresenceMember,
5+
type CollabSnapshot,
6+
} from '../src/collab/collab-client.js';
37
import { workspaceContextFixture } from './helpers/workspace-context';
48

59
const TEAM_CONTEXT = workspaceContextFixture({
@@ -15,12 +19,16 @@ interface RecordedCall {
1519
}
1620

1721
interface FakeFetchOptions {
18-
present?: Array<{ memberId: string; name?: string }>;
22+
present?: CollabPresenceMember[];
1923
publishedVersion?: number | null;
2024
syncState?: string | null;
2125
failPath?: string;
2226
}
2327

28+
const PRESENCE_TEST_NOW = Date.parse('2026-08-05T00:00:00.000Z');
29+
const presenceTime = (offsetMs = 0) =>
30+
new Date(PRESENCE_TEST_NOW + offsetMs).toISOString();
31+
2432
function makeFetch(options: FakeFetchOptions = {}) {
2533
const calls: RecordedCall[] = [];
2634
const state = {
@@ -54,6 +62,7 @@ function makeFetch(options: FakeFetchOptions = {}) {
5462

5563
beforeEach(() => {
5664
vi.useFakeTimers();
65+
vi.setSystemTime(PRESENCE_TEST_NOW);
5766
});
5867

5968
afterEach(() => {
@@ -627,11 +636,11 @@ describe('CollabClient', () => {
627636
client.stop();
628637
});
629638

630-
it('retains a peer for one heartbeat window when one self-bearing roster omits it', async () => {
639+
it('retains a peer for the upstream lease window when self-bearing rosters briefly omit it', async () => {
631640
const { fetchImpl, state } = makeFetch({
632641
present: [
633-
{ memberId: 'viewer', name: 'Viewer' },
634-
{ memberId: 'peer', name: 'Peer' },
642+
{ memberId: 'viewer', name: 'Viewer', heartbeatAt: presenceTime() },
643+
{ memberId: 'peer', name: 'Peer', heartbeatAt: presenceTime() },
635644
],
636645
});
637646
const client = new CollabClient({
@@ -646,20 +655,154 @@ describe('CollabClient', () => {
646655
await vi.advanceTimersByTimeAsync(0);
647656
expect(client.getSnapshot().present).toHaveLength(2);
648657

649-
state.present = [{ memberId: 'viewer', name: 'Viewer' }];
658+
state.present = [{
659+
memberId: 'viewer',
660+
name: 'Viewer',
661+
heartbeatAt: presenceTime(10_000),
662+
}];
650663
await vi.advanceTimersByTimeAsync(10_000);
651-
expect(client.getSnapshot().present).toEqual([
652-
{ memberId: 'viewer', name: 'Viewer' },
653-
{ memberId: 'peer', name: 'Peer' },
654-
]);
664+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
665+
.toEqual(['viewer', 'peer']);
666+
667+
// The Vela presence lease is authoritative for 30 seconds from the last
668+
// roster that actually contained the peer. A delayed heartbeat can make
669+
// the peer absent from the 10s and 20s reads without making that last-good
670+
// evidence stale. Dropping it after one interval makes the non-owner side
671+
// flicker while the owner's local self witness hides the same upstream gap.
672+
await vi.advanceTimersByTimeAsync(19_999);
673+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
674+
.toEqual(['viewer', 'peer']);
675+
676+
// At the exact upstream TTL boundary the peer is no longer retained.
677+
await vi.advanceTimersByTimeAsync(1);
678+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
679+
.toEqual(['viewer']);
680+
681+
client.stop();
682+
});
683+
684+
it('never flashes a known peer when it returns before the upstream lease window ends', async () => {
685+
const { fetchImpl, state } = makeFetch({
686+
present: [
687+
{ memberId: 'viewer', name: 'Viewer', heartbeatAt: presenceTime() },
688+
{ memberId: 'owner', name: 'Owner', heartbeatAt: presenceTime() },
689+
],
690+
});
691+
const snapshots: CollabPresenceMember[][] = [];
692+
const client = new CollabClient({
693+
projectId: 'p-owner-transient-gap',
694+
member: { memberId: 'viewer', name: 'Viewer' },
695+
fetch: fetchImpl,
696+
heartbeatMs: 10_000,
697+
onUpdate: (snapshot) => snapshots.push(snapshot.present),
698+
});
655699

656-
await vi.advanceTimersByTimeAsync(9_999);
700+
client.start();
701+
await vi.advanceTimersByTimeAsync(0);
702+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
703+
.toEqual(['viewer', 'owner']);
704+
snapshots.length = 0;
705+
state.present = [{
706+
memberId: 'viewer',
707+
name: 'Viewer',
708+
heartbeatAt: presenceTime(10_000),
709+
}];
710+
await vi.advanceTimersByTimeAsync(20_000);
711+
state.present = [
712+
{ memberId: 'viewer', name: 'Viewer', heartbeatAt: presenceTime(30_000) },
713+
{ memberId: 'owner', name: 'Owner', heartbeatAt: presenceTime(30_000) },
714+
];
715+
await vi.advanceTimersByTimeAsync(10_000);
716+
717+
expect(snapshots.every((present) =>
718+
present.some(({ memberId }) => memberId === 'owner'))).toBe(true);
657719
expect(client.getSnapshot().present).toHaveLength(2);
720+
client.stop();
721+
});
722+
723+
it('does not extend a nearly expired backend lease from the local observation time', async () => {
724+
vi.setSystemTime(PRESENCE_TEST_NOW + 29_000);
725+
const { fetchImpl, state } = makeFetch({
726+
present: [
727+
{ memberId: 'viewer', heartbeatAt: presenceTime(29_000) },
728+
{ memberId: 'owner', heartbeatAt: presenceTime() },
729+
],
730+
});
731+
const client = new CollabClient({
732+
projectId: 'p-nearly-expired-owner',
733+
member: { memberId: 'viewer' },
734+
fetch: fetchImpl,
735+
heartbeatMs: 1_000,
736+
});
737+
738+
client.start();
739+
await vi.advanceTimersByTimeAsync(0);
740+
state.present = [{
741+
memberId: 'viewer',
742+
heartbeatAt: presenceTime(30_000),
743+
}];
744+
await vi.advanceTimersByTimeAsync(999);
745+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
746+
.toEqual(['viewer', 'owner']);
747+
658748
await vi.advanceTimersByTimeAsync(1);
749+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
750+
.toEqual(['viewer']);
751+
client.stop();
752+
});
753+
754+
it.each([
755+
['missing', undefined],
756+
['invalid', 'not-a-date'],
757+
])('retains a peer with %s heartbeatAt for only one fallback heartbeat window', async (_label, heartbeatAt) => {
758+
const { fetchImpl, state } = makeFetch({
759+
present: [
760+
{ memberId: 'viewer' },
761+
{ memberId: 'legacy-peer', heartbeatAt },
762+
],
763+
});
764+
const client = new CollabClient({
765+
projectId: 'p-legacy-presence',
766+
member: { memberId: 'viewer' },
767+
fetch: fetchImpl,
768+
heartbeatMs: 10_000,
769+
});
770+
771+
client.start();
772+
await vi.advanceTimersByTimeAsync(0);
773+
state.present = [{ memberId: 'viewer' }];
774+
await vi.advanceTimersByTimeAsync(19_999);
775+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
776+
.toEqual(['viewer', 'legacy-peer']);
777+
778+
await vi.advanceTimersByTimeAsync(1);
779+
expect(client.getSnapshot().present.map(({ memberId }) => memberId))
780+
.toEqual(['viewer']);
781+
client.stop();
782+
});
783+
784+
it('applies an event-driven fresh roster exactly so explicit leaves stay immediate', async () => {
785+
const { fetchImpl, state } = makeFetch({
786+
present: [
787+
{ memberId: 'viewer', name: 'Viewer' },
788+
{ memberId: 'peer', name: 'Peer' },
789+
],
790+
});
791+
const client = new CollabClient({
792+
projectId: 'p-explicit-leave',
793+
member: { memberId: 'viewer', name: 'Viewer' },
794+
fetch: fetchImpl,
795+
heartbeatMs: 10_000,
796+
});
797+
798+
client.start();
799+
await vi.advanceTimersByTimeAsync(0);
800+
state.present = [{ memberId: 'viewer', name: 'Viewer' }];
801+
await client.refreshPresence();
802+
659803
expect(client.getSnapshot().present).toEqual([
660804
{ memberId: 'viewer', name: 'Viewer' },
661805
]);
662-
663806
client.stop();
664807
});
665808

0 commit comments

Comments
 (0)