[Bug] You can see (and try to call) yourself in the video-call user picker — chatService.userId non-reactive race #98

New Issue

2026-05-10T23:42:40Z

spikerj commented

2026-05-10 23:42:40 +00:00

Symptom

When opening Start a Video Call from the user menu, the current user sometimes appears in the "who do you want to call" list — you can pick yourself and try to invite yourself.

Observed on learn.spikersoft.com:

From spikerj account: list shows Joseph Spiker (self) AND Bobby Spiker.
From bspiker account: list correctly shows only Joseph Spiker (no self).

So the bug is intermittent and depends on async timing — same code path, two different outcomes on two accounts.

Repro

Log in as any user.
Open Start a Video Call from the user menu.
If profile load loses the race against the connected-users payload, you'll see your own row in the list.
Refreshing may or may not change the result — it's a startup race.

Root cause

ChatService.userId is a plain string field, not a signal:

public userId: string = "";

...mutated imperatively after loadUserProfile() resolves. Reactive consumers like the picker's allUsers = computed(...) and the chat panel's onlineUsers / offlineUsers / currentChatTitle computeds read it inside their compute functions, but computed() only re-runs when something it tracks (signals/observables) changes. It tracks users() (real signal) but not userId (raw property). So:

Picker opens. users() is empty, userId === "", computed returns [].
Hub emits GetConnectedUsers payload. users.set([...]) fires — computed re-runs.
At this moment userId may still be "" (profile load hasn't resolved). The filter predicate u.id !== "" is true for every user, so self is included.
loadUserProfile() finally resolves, sets userId = profile.id. Computed never re-runs because nothing it subscribes to changed.

On bspiker the profile load happened to win the race; on spikerj it didn't.

Same latent bug in chat.component.ts — onlineUsers, offlineUsers, and currentChatTitle read chatService.userId from inside computed() blocks too.

Impact

Severity: P2 — you can technically invite yourself; the call would never connect (peer ID is the same), but it's a confusing UX bug and looks broken.
Surface: video-call picker + main chat panel user lists.
Repro reliability: intermittent, race-dependent.

How we'll prevent regressions

Docstring on the new userIdSig signal explicitly tells future contributors to use it inside computed() / templates instead of the plain string field, and explains the failure mode this fix closed.

## Symptom When opening `Start a Video Call` from the user menu, the current user sometimes appears in the "who do you want to call" list — you can pick yourself and try to invite yourself. Observed on `learn.spikersoft.com`: - From `spikerj` account: list shows **Joseph Spiker (self) AND Bobby Spiker**. - From `bspiker` account: list correctly shows **only Joseph Spiker** (no self). So the bug is intermittent and depends on async timing — same code path, two different outcomes on two accounts. ## Repro 1. Log in as any user. 2. Open `Start a Video Call` from the user menu. 3. If profile load loses the race against the connected-users payload, you'll see your own row in the list. 4. Refreshing may or may not change the result — it's a startup race. ## Root cause `ChatService.userId` is a **plain string field**, not a signal: ```ts public userId: string = ""; ``` ...mutated imperatively after `loadUserProfile()` resolves. Reactive consumers like the picker's `allUsers = computed(...)` and the chat panel's `onlineUsers / offlineUsers / currentChatTitle` computeds read it inside their compute functions, but `computed()` only re-runs when something it **tracks** (signals/observables) changes. It tracks `users()` (real signal) but **not** `userId` (raw property). So: 1. Picker opens. `users()` is empty, `userId === ""`, computed returns `[]`. 2. Hub emits `GetConnectedUsers` payload. `users.set([...])` fires — computed re-runs. 3. At this moment `userId` may still be `""` (profile load hasn't resolved). The filter predicate `u.id !== ""` is true for **every** user, so self is included. 4. `loadUserProfile()` finally resolves, sets `userId = profile.id`. Computed never re-runs because nothing it subscribes to changed. On `bspiker` the profile load happened to win the race; on `spikerj` it didn't. Same latent bug in `chat.component.ts` — `onlineUsers`, `offlineUsers`, and `currentChatTitle` read `chatService.userId` from inside `computed()` blocks too. ## Impact - **Severity:** P2 — you can technically invite yourself; the call would never connect (peer ID is the same), but it's a confusing UX bug and looks broken. - **Surface:** video-call picker + main chat panel user lists. - **Repro reliability:** intermittent, race-dependent. ## How we'll prevent regressions Docstring on the new `userIdSig` signal explicitly tells future contributors to use it inside `computed()` / templates instead of the plain string field, and explains the failure mode this fix closed.

spikerj added the bug label 2026-05-10 23:42:47 +00:00

spikerj commented

2026-05-10 23:43:33 +00:00

Resolution

Fixed by adding a reactive mirror of userId on ChatService and switching the racy reactive consumers to it.

`ChatService` — add a signal mirror

public userId: string = "";
public userName: string = "";

/**
 * Reactive mirror of `userId`. Use this from `computed()` /
 * templates instead of the plain string field — the field is mutated
 * imperatively after profile load, so a `computed` that reads it won't
 * re-evaluate when it finally lands. Reading the signal subscribes the
 * computation to the change, which closed the "I see myself in the
 * call-picker user list" bug where the user list arrived before the
 * profile resolved.
 */
public readonly userIdSig = signal<string>("");

Both write sites (initializeService after first profile load, and the public refreshUserProfile()) now userIdSig.set(this.userId) alongside the legacy field assignment so the signal stays in sync.

Reactive consumers switched to the signal

video-call-picker.component.ts:

public readonly allUsers = computed(() => {
    const selfId = this.chatService.userIdSig();
    return this.chatService.users().filter((u) => u.id !== selfId);
});

chat.component.ts — onlineUsers, offlineUsers, and currentChatTitle were rewritten the same way (read userIdSig() once at the top of each computed, then filter).

Imperative call sites that read chatService.userId (event handlers, getUserMessageHistory, etc.) are unchanged because they only fire after user interaction — by which time the field is already populated and there's no timing race to worry about. Keeping the change surface small avoided needless .userId → .userId() churn across unrelated sites.

Why this closes the bug for good

Reading userIdSig() inside a computed subscribes the computation to identity changes. So whichever event lands last — the user-list payload OR the profile-load resolve — forces the computed to re-run with both pieces in hand. There's no longer a window where the filter sees "" and matches everyone.

Verification

Existing chat / picker specs still pass after the change.
Manual: in both account orderings (profile-first and users-first), the picker now consistently excludes self.

Closing as fixed.

## Resolution Fixed by adding a reactive mirror of `userId` on `ChatService` and switching the racy reactive consumers to it. ### `ChatService` — add a signal mirror ```ts public userId: string = ""; public userName: string = ""; /** * Reactive mirror of `userId`. Use this from `computed()` / * templates instead of the plain string field — the field is mutated * imperatively after profile load, so a `computed` that reads it won't * re-evaluate when it finally lands. Reading the signal subscribes the * computation to the change, which closed the "I see myself in the * call-picker user list" bug where the user list arrived before the * profile resolved. */ public readonly userIdSig = signal<string>(""); ``` Both write sites (`initializeService` after first profile load, and the public `refreshUserProfile()`) now `userIdSig.set(this.userId)` alongside the legacy field assignment so the signal stays in sync. ### Reactive consumers switched to the signal **`video-call-picker.component.ts`:** ```ts public readonly allUsers = computed(() => { const selfId = this.chatService.userIdSig(); return this.chatService.users().filter((u) => u.id !== selfId); }); ``` **`chat.component.ts`** — `onlineUsers`, `offlineUsers`, and `currentChatTitle` were rewritten the same way (read `userIdSig()` once at the top of each `computed`, then filter). Imperative call sites that read `chatService.userId` (event handlers, `getUserMessageHistory`, etc.) are unchanged because they only fire after user interaction — by which time the field is already populated and there's no timing race to worry about. Keeping the change surface small avoided needless `.userId` → `.userId()` churn across unrelated sites. ### Why this closes the bug for good Reading `userIdSig()` inside a `computed` subscribes the computation to identity changes. So whichever event lands last — the user-list payload OR the profile-load resolve — forces the computed to re-run with both pieces in hand. There's no longer a window where the filter sees `""` and matches everyone. ### Verification - Existing chat / picker specs still pass after the change. - Manual: in both account orderings (profile-first and users-first), the picker now consistently excludes self. Closing as fixed.

spikerj closed this issue

2026-05-10 23:43:44 +00:00

spikerj referenced this issue

2026-07-02 15:06:59 +00:00

feat(my-journey): winding-path progression visualization — optional, menu-launched (frontend) #333

spikerj referenced this issue

2026-07-02 15:07:50 +00:00

[Epic] Playground uniformity & lesson content layers (C / C++ / JavaScript / Python / C#) #323

spikerj referenced this issue

2026-07-02 15:07:59 +00:00

[Spike] Duolingo-style progression visualization — optional, menu-launched, sounds, younger audience #330

spikerj referenced this issue