Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,22 @@
# Bot review rounds: two, then stop.
#
# CodeRabbit runs on the ASSERTIVE profile and re-reviews on every push, so each
# round of fixes produces a fresh round of comments on the diff those fixes
# created. Round 1 clears the substantive findings; round 2 clears whatever
# round 1 provoked. After that, answer remaining comments in the thread and
# merge — do not push again just to silence the bot.
#
# The drawback, stated so nobody has to rediscover it: stopping at two means a
# genuine finding raised in round 3 gets a reply rather than a fix in this PR.
# That is the accepted cost. The alternative — looping until the bot is silent —
# does not converge on an ASSERTIVE profile: it reliably finds something on any
# new diff, and past round 2 that something is overwhelmingly refactor-for-its-
# own-sake (cognitive-complexity thresholds, helper extraction). Each extra push
# also resets approvals and burns a full CI cycle, so the churn is not free.
#
# Anything real that surfaces late belongs in a follow-up issue, where it keeps
# its own context, instead of growing this PR's diff past what a human will read.

name: CI

on:
Expand Down
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,8 @@ node-payment-main/
.claude/worktrees/
screens/
prompts/

# Pre-images written by scripts/stream/*.ts before they mutate the live Stream
# app. These are snapshots of production configuration (role grants, channel
# rosters) and rollback material for an operator's machine — not source.
.stream-backups/
256 changes: 256 additions & 0 deletions __tests__/chat/channel-search-race.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
/**
* Search told two lies, and both were about time rather than data.
*
* 1. "No results found for michael" rendered while Michael Chen sat in the list
* underneath. The empty state was gated on `!loading`, but `setLoading(true)`
* runs INSIDE the search, which fires 300ms after the keystroke — so for that
* whole window `loading` was false and no request existed yet. It announced
* failure before it had looked, on every keystroke.
*
* 2. An empty search box with a stale result still listed. There was no
* `AbortController` and no latest-wins guard, so `setSearchResults` committed
* whichever response landed last regardless of which query it answered.
*
* Both are invisible to a test that resolves fetches in order, which is why
* these deliberately resolve them OUT of order and assert on the settled DOM.
*/

// Silences React's "not wrapped in act" warning; every render here is.
(
globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }
).IS_REACT_ACT_ENVIRONMENT = true;

const setActiveChannel = jest.fn();
const openConversation = jest.fn();

jest.mock("stream-chat-react", () => ({
useChatContext: () => ({
client: { userID: "me", channel: jest.fn() },
setActiveChannel,
}),
}));
jest.mock("../../components/chat/ChatPaneContext", () => ({
useChatPane: () => ({ openConversation }),
}));
jest.mock("next/image", () => ({
__esModule: true,
default: () => null,
}));

import React, { act } from "react";
import { createRoot, type Root } from "react-dom/client";

import { ChannelSearch } from "../../components/chat/ChannelSearch";

let host: HTMLDivElement;
let root: Root;

/** A consultation row as the search route returns it. */
function row(name: string, channelId: string) {
return {
id: `${channelId}-id`,
type: "consultation" as const,
name: `${name}'s plan`,
counterpartyName: name,
counterpartyUserId: `${channelId}-user`,
organizationId: null,
channelId,
};
}

/** A fetch whose responses resolve only when the test says so. */
function deferredFetch() {
const pending: Array<{
query: string;
resolve: (rows: ReturnType<typeof row>[]) => void;
}> = [];

const fetchMock = jest.fn((url: string, init?: RequestInit) => {
const query = new URL(url, "http://localhost").searchParams.get("q") ?? "";
return new Promise((resolve, reject) => {
// `new DOMException(msg, name)` already sets `.name`. Do NOT then
// `Object.assign` it — `name` is a getter-only accessor on the prototype,
// so writing to it throws a TypeError in strict mode, the listener dies,
// the promise never rejects, and the test "fails" against perfectly
// correct component code.
init?.signal?.addEventListener("abort", () =>
reject(new DOMException("aborted", "AbortError")),
);
pending.push({
query,
resolve: (rows) =>
resolve({ ok: true, json: async () => rows } as Response),
});
});
});

return { fetchMock, pending };
}

async function type(value: string) {
const input = host.querySelector("input") as HTMLInputElement;
await act(async () => {
const setter = Object.getOwnPropertyDescriptor(
window.HTMLInputElement.prototype,
"value",
)!.set!;
setter.call(input, value);
input.dispatchEvent(new Event("input", { bubbles: true }));
});
}

/** Advance past the 300ms debounce and let promises flush. */
async function settleDebounce() {
await act(async () => {
jest.advanceTimersByTime(350);
await Promise.resolve();
});
}

beforeEach(() => {
jest.useFakeTimers();
host = document.createElement("div");
document.body.appendChild(host);
root = createRoot(host);
});

afterEach(() => {
act(() => root.unmount());
host.remove();
jest.useRealTimers();
jest.restoreAllMocks();
});

describe("the empty state does not fire before a search has run", () => {
it("stays silent during the debounce window", async () => {
const { fetchMock } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));
await type("michael");

// Mid-debounce: the request has not been issued yet.
expect(fetchMock).not.toHaveBeenCalled();
// This is the reported bug — the box claimed there was nothing to find
// before it had looked.
expect(host.textContent).not.toContain("No results found");
});

it("stays silent while the request is in flight", async () => {
const { fetchMock } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));
await type("michael");
await settleDebounce();

expect(fetchMock).toHaveBeenCalledTimes(1);
expect(host.textContent).not.toContain("No results found");
});

it("shows it only once a search has genuinely returned nothing", async () => {
const { fetchMock, pending } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));
await type("michael");
await settleDebounce();

await act(async () => {
pending[0].resolve([]);
await Promise.resolve();
});

expect(host.textContent).toContain("No results found");
});
});

describe("out-of-order responses", () => {
it("ignores a stale response that lands after a newer one", async () => {
const { fetchMock, pending } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));

await type("chen");
await settleDebounce();
await type("michael");
await settleDebounce();

expect(pending).toHaveLength(2);
expect(pending[0].query).toBe("chen");
expect(pending[1].query).toBe("michael");

// Newer first, then the stale one — the order that used to repaint the
// dropdown with results for a query the user had already moved off.
await act(async () => {
pending[1].resolve([row("Michael Chen", "dm-a-michael")]);
await Promise.resolve();
});
await act(async () => {
pending[0].resolve([row("Samantha Chen", "dm-a-samantha")]);
await Promise.resolve();
});

expect(host.textContent).toContain("Michael Chen");
expect(host.textContent).not.toContain("Samantha Chen");
});

it("aborts the previous request when a new one starts", async () => {
const { fetchMock } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));

await type("chen");
await settleDebounce();
await type("michael");
await settleDebounce();

const firstSignal = (fetchMock.mock.calls[0][1] as RequestInit).signal;
expect(firstSignal?.aborted).toBe(true);
});

it("does not repopulate the dropdown after the query is cleared", async () => {
const { fetchMock, pending } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));

await type("samantha");
await settleDebounce();

// Clear the box, then let the earlier response arrive. This is the reported
// screenshot: an empty search field with a result still listed under it.
await type("");
await settleDebounce();

await act(async () => {
pending[0].resolve([row("Samantha Chen", "dm-a-samantha")]);
await Promise.resolve();
});

expect(host.textContent).not.toContain("Samantha Chen");
});
});

describe("what matched is visible", () => {
it("shows the plan title, not just the counterparty", async () => {
const { fetchMock, pending } = deferredFetch();
global.fetch = fetchMock as unknown as typeof fetch;

await act(async () => root.render(<ChannelSearch />));
await type("michael");
await settleDebounce();

// The route matches plan titles too, so a row can match on something the
// UI never displayed — which reads as a wrong result.
await act(async () => {
pending[0].resolve([row("Robert Brown", "dm-a-robert")]);
await Promise.resolve();
});

expect(host.textContent).toContain("Robert Brown");
expect(host.textContent).toContain("Robert Brown's plan");
});
});
103 changes: 103 additions & 0 deletions __tests__/chat/phantom-dm-filtering.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/**
* A DM with nobody on the other end must not be reachable.
*
* The first attempt at this only changed the LABEL: `channelUtils` stopped
* printing the raw channel id and printed "Unavailable conversation" instead.
* That is cosmetic, and it showed — the row was still in the list, still
* clickable, still accepted a message. Renaming a broken thing does not fix it.
*
* `isUsableDmChannel` is the predicate the sidebar filters on, so a phantom
* never reaches the rendered list at all. These tests pin the two properties
* that matter: it catches every under-populated `messaging` channel, and it
* leaves `team` channels alone — a webinar channel legitimately holds only its
* host between creation and the first registration, and filtering those would
* hide working event chats.
*/
import type { Channel } from "stream-chat";

import {
getChannelDisplayInfo,
isUsableDmChannel,
} from "@/components/chat/utils/channelUtils";

/** Minimal stand-in — only the fields the predicate and the labeller read. */
function fakeChannel(
type: "messaging" | "team",
memberIds: string[],
data: Record<string, unknown> = {},
): Channel {
return {
type,
id: `${type}-fixture`,
cid: `${type}:fixture`,
data,
state: {
members: Object.fromEntries(
memberIds.map((id) => [id, { user: { id, name: `User ${id}` } }]),
),
},
} as unknown as Channel;
}

describe("isUsableDmChannel", () => {
it("rejects a messaging channel with no members", () => {
// The `watch()`-created phantom: `created_by` is set, membership is empty.
expect(isUsableDmChannel(fakeChannel("messaging", []))).toBe(false);
});

it("rejects a messaging channel with only the viewer", () => {
expect(isUsableDmChannel(fakeChannel("messaging", ["me"]))).toBe(false);
});

it("accepts a real two-person DM", () => {
expect(isUsableDmChannel(fakeChannel("messaging", ["me", "them"]))).toBe(
true,
);
});

it("accepts a group DM", () => {
expect(isUsableDmChannel(fakeChannel("messaging", ["me", "a", "b"]))).toBe(
true,
);
});

it("leaves a one-member team channel alone", () => {
// A webinar channel is created with its host before anyone registers.
// Applying the DM rule here would hide a working event chat.
expect(isUsableDmChannel(fakeChannel("team", ["host"]))).toBe(true);
});

it("leaves an empty team channel alone", () => {
expect(isUsableDmChannel(fakeChannel("team", []))).toBe(true);
});

it("survives a channel whose state has not loaded", () => {
const bare = {
type: "messaging",
id: "x",
cid: "messaging:x",
} as unknown as Channel;
expect(isUsableDmChannel(bare)).toBe(false);
});
});

describe("the label is the last resort, not the fix", () => {
it("never renders a raw channel id for a broken DM", () => {
const info = getChannelDisplayInfo(fakeChannel("messaging", ["me"]), "me");

// The reported symptom was a header reading `dm-cmqb1680e0014txyocn1f0dbz-…`.
// A channel id is an internal key: it is not a name, and it leaks both
// participants' user ids into the UI.
expect(info.displayName).not.toMatch(/^messaging-/);
expect(info.displayName).toBe("Unavailable conversation");
expect(info.statusText).toBe("No other participants");
});

it("still names the counterparty on a healthy DM", () => {
const info = getChannelDisplayInfo(
fakeChannel("messaging", ["me", "them"]),
"me",
);
expect(info.displayName).toBe("User them");
});
});
Loading
Loading