Skip to content

Commit 0bcbca4

Browse files
committed
test(webhooks): cover relay client and harden splitCommaList empty handling
- splitCommaList now returns undefined for empty/whitespace-only values so callers treat them as "not provided" rather than sending an empty array - list now prints the iterator hint when paginating - add relay-client tests plus list/replay/update/verify coverage - README: clarify keepalive probe timing and JSON-mode type discriminator Claude-Session: https://claude.ai/code/session_01Mwcxk4pmfNYtmvjwWs9jUE
1 parent 5fad1c7 commit 0bcbca4

8 files changed

Lines changed: 303 additions & 4 deletions

File tree

packages/cli-core/src/commands/webhooks/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,10 @@ clerk webhooks listen [--forward-to <url>] [--events <list>] [--skip-verify] [--
247247
Behavior notes:
248248
249249
- **Relay token**: `c_` + 10 random base62 chars — the same token goes in the start frame, the inbox URL, and the per-instance CLI config (`relay.<instanceId>.token`). Live-relay verified: `play.svix.com` rejects unprefixed tokens, and the relay only registers an inbox when the start frame carries the `c_` token. Close code 1008 = token collision → new token generated, persisted, redialed, and the endpoint URL re-pointed.
250-
- **Keepalive**: the relay server pings ~every 21s, but Bun's client WebSocket auto-pongs below the JS API (no ping events). After 30s of silence the client sends an active `ws.ping()` probe — writes to a dead link surface as close/error, which redials with the same token. Reconnects never change the relay URL.
250+
- **Keepalive**: the relay server pings ~every 21s, but Bun's client WebSocket auto-pongs below the JS API (no ping events). A probe timer fires every `RELAY_SILENCE_TIMEOUT_MS / 2` (~15s) and, once at least `RELAY_SILENCE_TIMEOUT_MS` (30s) has elapsed with no inbound message, sends an active `ws.ping()` (so a probe lands 30–45s into a silence, depending on timer phase) — writes to a dead link surface as close/error, which redials with the same token. Reconnects never change the relay URL.
251251
- **Per-delivery output**: human mode prints `time --> event_type msg_…` then `<-- status method path ms` via `log.ui` (bypasses the stderr throttle). Diagnostics: 401 → `clerkMiddleware` public-route hint; 400 → raw-body/`verifyWebhook()` order hint; 5xx → response body inline plus the exact `clerk webhooks replay <msg_id>` line; unreachable handler → synthetic **502** framed back to the relay.
252252
- **Verification**: deliveries failing HMAC are warned about and still forwarded (the mismatch means the relay secret diverged, not that the local handler should silently miss events).
253-
- **Agent/`--json` mode**: NDJSON on stdoutone `ready` line (`relay_url`, `signing_secret`, `endpoint_id`, `events_filter`), then one `event` line per delivery (`svix_id`, `event_type`, `headers`, `body_b64`, `forward_status`, `latency_ms`). An event line saved to a file is directly consumable by `verify --delivery @file`.
253+
- **Agent/`--json` mode**: NDJSON on stdout. Every line carries a `type` discriminator: one `{ "type": "ready", ... }` line (`relay_url`, `signing_secret`, `endpoint_id`, `events_filter`), then one `{ "type": "event", ... }` line per delivery (`svix_id`, `event_type`, `headers`, `body_b64`, `forward_status`, `latency_ms`). An event line saved to a file is directly consumable by `verify --delivery @file`.
254254
- **SIGINT**: `listen` replaces the global cleanup-free handler before opening the socket: close socket, drain in-flight forwards, exit 130. The relay endpoint is **never** deleted on exit — its URL and `whsec_` stay stable across restarts. `listen` never exits 0.
255255
256256
### API endpoints

packages/cli-core/src/commands/webhooks/list.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,15 @@ describe("webhooks list", () => {
130130
expect(captured.err).toContain("--iterator iter_next");
131131
});
132132

133+
test("still hints at the next --iterator on an empty page with has_next_page", async () => {
134+
mockListWebhookEndpoints.mockResolvedValue(listResponse({ data: [], has_next_page: true }));
135+
136+
await webhooksList();
137+
138+
expect(captured.err).toContain("No webhook endpoints found.");
139+
expect(captured.err).toContain("--iterator iter_next");
140+
});
141+
133142
test("omits the pagination hint on the last page", async () => {
134143
await webhooksList();
135144

packages/cli-core/src/commands/webhooks/list.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export async function webhooksList(options: WebhooksListOptions = {}): Promise<v
5959

6060
if (response.data.length === 0) {
6161
log.warn("No webhook endpoints found.");
62+
printIteratorHint(response.cursor);
6263
return;
6364
}
6465

Lines changed: 257 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,257 @@
1+
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
2+
import { useCaptureLog } from "../../test/lib/stubs.ts";
3+
import { RelayClient } from "./relay-client.ts";
4+
import {
5+
RELAY_CLOSE_TOKEN_COLLISION,
6+
RELAY_RECONNECT_DELAY_MS,
7+
RELAY_SILENCE_TIMEOUT_MS,
8+
encodeStartFrame,
9+
} from "./relay-protocol.ts";
10+
11+
/**
12+
* Stand-in for Bun's client WebSocket. Records what the client sends/pings,
13+
* and exposes manual `open()`/`message()`/`fireClose()` triggers so tests drive
14+
* the connection lifecycle without a real socket.
15+
*/
16+
class FakeWebSocket {
17+
static OPEN = 1;
18+
static instances: FakeWebSocket[] = [];
19+
20+
readyState = FakeWebSocket.OPEN;
21+
onopen: (() => void) | null = null;
22+
onmessage: ((event: { data: string }) => void) | null = null;
23+
onerror: (() => void) | null = null;
24+
onclose: ((event: { code: number }) => void) | null = null;
25+
sent: string[] = [];
26+
pingCount = 0;
27+
closedWith: number | undefined;
28+
pingThrows = false;
29+
30+
constructor(readonly url: string) {
31+
FakeWebSocket.instances.push(this);
32+
}
33+
34+
send(frame: string): void {
35+
this.sent.push(frame);
36+
}
37+
38+
ping(): void {
39+
if (this.pingThrows) throw new Error("dead link");
40+
this.pingCount++;
41+
}
42+
43+
close(code?: number): void {
44+
this.closedWith = code;
45+
}
46+
47+
open(): void {
48+
this.onopen?.();
49+
}
50+
51+
message(data: string): void {
52+
this.onmessage?.({ data });
53+
}
54+
55+
fireClose(code: number): void {
56+
this.onclose?.({ code });
57+
}
58+
}
59+
60+
const flush = () => new Promise<void>((resolve) => setImmediate(resolve));
61+
62+
/** Fetch a constructed socket, asserting it exists (satisfies noUncheckedIndexedAccess). */
63+
function wsAt(index: number): FakeWebSocket {
64+
const ws = FakeWebSocket.instances[index];
65+
if (!ws) throw new Error(`expected a relay socket at index ${index}`);
66+
return ws;
67+
}
68+
69+
// Captured timer callbacks so tests can invoke them on demand instead of waiting.
70+
let intervalCallback: (() => void) | undefined;
71+
let intervalDelay: number | undefined;
72+
let timeoutCallback: (() => void) | undefined;
73+
let timeoutDelay: number | undefined;
74+
let now = 0;
75+
76+
const realWebSocket = globalThis.WebSocket;
77+
const realSetInterval = globalThis.setInterval;
78+
const realClearInterval = globalThis.clearInterval;
79+
const realSetTimeout = globalThis.setTimeout;
80+
const realNow = Date.now;
81+
82+
describe("RelayClient", () => {
83+
useCaptureLog();
84+
85+
beforeEach(() => {
86+
FakeWebSocket.instances = [];
87+
intervalCallback = undefined;
88+
intervalDelay = undefined;
89+
timeoutCallback = undefined;
90+
timeoutDelay = undefined;
91+
now = 1_000_000;
92+
93+
(globalThis as unknown as { WebSocket: unknown }).WebSocket = FakeWebSocket;
94+
Date.now = () => now;
95+
globalThis.setInterval = ((fn: () => void, delay?: number) => {
96+
intervalCallback = fn;
97+
intervalDelay = delay;
98+
return 1 as unknown as ReturnType<typeof setInterval>;
99+
}) as typeof setInterval;
100+
globalThis.clearInterval = (() => {}) as typeof clearInterval;
101+
globalThis.setTimeout = ((fn: () => void, delay?: number) => {
102+
timeoutCallback = fn;
103+
timeoutDelay = delay;
104+
return 2 as unknown as ReturnType<typeof setTimeout>;
105+
}) as unknown as typeof setTimeout;
106+
});
107+
108+
afterEach(() => {
109+
(globalThis as unknown as { WebSocket: unknown }).WebSocket = realWebSocket;
110+
globalThis.setInterval = realSetInterval;
111+
globalThis.clearInterval = realClearInterval;
112+
globalThis.setTimeout = realSetTimeout;
113+
Date.now = realNow;
114+
});
115+
116+
function makeClient(overrides: Partial<ConstructorParameters<typeof RelayClient>[0]> = {}) {
117+
const events: Array<{ id: string }> = [];
118+
const rotated: string[] = [];
119+
let reconnects = 0;
120+
const client = new RelayClient({
121+
token: "c_original00",
122+
url: "ws://relay.test",
123+
onEvent: (event) => events.push(event),
124+
onTokenRotated: async (token) => {
125+
rotated.push(token);
126+
},
127+
onReconnect: () => {
128+
reconnects++;
129+
},
130+
...overrides,
131+
});
132+
return {
133+
client,
134+
events,
135+
rotated,
136+
get reconnects() {
137+
return reconnects;
138+
},
139+
};
140+
}
141+
142+
async function openClient(overrides?: Partial<ConstructorParameters<typeof RelayClient>[0]>) {
143+
const harness = makeClient(overrides);
144+
const started = harness.client.start();
145+
wsAt(0).open();
146+
await started;
147+
return harness;
148+
}
149+
150+
test("start() dials the override URL and sends the c_-prefixed start frame", async () => {
151+
const { client } = await openClient();
152+
const ws = wsAt(0);
153+
154+
expect(ws.url).toBe("ws://relay.test");
155+
expect(ws.sent[0]).toBe(encodeStartFrame("c_original00"));
156+
expect(client.token).toBe("c_original00");
157+
});
158+
159+
test("schedules the keepalive probe at RELAY_SILENCE_TIMEOUT_MS / 2", async () => {
160+
await openClient();
161+
expect(intervalDelay).toBe(RELAY_SILENCE_TIMEOUT_MS / 2);
162+
});
163+
164+
test("keepalive pings only after RELAY_SILENCE_TIMEOUT_MS of silence", async () => {
165+
await openClient();
166+
const ws = wsAt(0);
167+
168+
now += RELAY_SILENCE_TIMEOUT_MS - 1; // still within the window
169+
intervalCallback?.();
170+
expect(ws.pingCount).toBe(0);
171+
172+
now += 2; // now past the silence threshold
173+
intervalCallback?.();
174+
expect(ws.pingCount).toBe(1);
175+
});
176+
177+
test("an inbound message resets the silence clock, deferring the next ping", async () => {
178+
const { events } = await openClient();
179+
const ws = wsAt(0);
180+
181+
now += RELAY_SILENCE_TIMEOUT_MS - 5;
182+
ws.message(
183+
JSON.stringify({
184+
type: "event",
185+
data: { id: "frm_1", method: "POST", headers: {}, body: "" },
186+
}),
187+
);
188+
expect(events).toHaveLength(1);
189+
190+
// Only 5ms have elapsed since the message reset lastActivityAt.
191+
now += 5;
192+
intervalCallback?.();
193+
expect(ws.pingCount).toBe(0);
194+
});
195+
196+
test("a dead-link ping closes the socket so the redial path fires", async () => {
197+
await openClient();
198+
const ws = wsAt(0);
199+
ws.pingThrows = true;
200+
201+
now += RELAY_SILENCE_TIMEOUT_MS + 1;
202+
intervalCallback?.();
203+
expect(ws.closedWith).toBeUndefined(); // close() called with no code on ping failure
204+
expect(ws.pingCount).toBe(0);
205+
});
206+
207+
test("a non-1008 close reconnects with the SAME token after the reconnect delay", async () => {
208+
const harness = await openClient();
209+
wsAt(0).fireClose(1006);
210+
211+
expect(harness.reconnects).toBe(1);
212+
expect(timeoutDelay).toBe(RELAY_RECONNECT_DELAY_MS);
213+
expect(FakeWebSocket.instances).toHaveLength(1); // no redial until the timer fires
214+
215+
timeoutCallback?.();
216+
expect(FakeWebSocket.instances).toHaveLength(2);
217+
wsAt(1).open();
218+
expect(wsAt(1).sent[0]).toBe(encodeStartFrame("c_original00"));
219+
expect(harness.rotated).toHaveLength(0);
220+
});
221+
222+
test("a 1008 collision rotates to a fresh c_ token, persists it, and redials", async () => {
223+
const harness = await openClient();
224+
wsAt(0).fireClose(RELAY_CLOSE_TOKEN_COLLISION);
225+
await flush();
226+
227+
expect(harness.client.token).not.toBe("c_original00");
228+
expect(harness.client.token).toMatch(/^c_[0-9A-Za-z]{10}$/);
229+
expect(harness.rotated).toEqual([harness.client.token]);
230+
231+
expect(FakeWebSocket.instances).toHaveLength(2); // immediate redial, no reconnect-delay timer
232+
wsAt(1).open();
233+
expect(wsAt(1).sent[0]).toBe(encodeStartFrame(harness.client.token));
234+
expect(harness.reconnects).toBe(0); // collision is not a generic reconnect
235+
});
236+
237+
test("stop() closes with 1000 and suppresses any further reconnect", async () => {
238+
const harness = await openClient();
239+
const ws = wsAt(0);
240+
241+
harness.client.stop();
242+
expect(ws.closedWith).toBe(1000);
243+
244+
ws.fireClose(1000);
245+
expect(harness.reconnects).toBe(0);
246+
expect(FakeWebSocket.instances).toHaveLength(1);
247+
});
248+
249+
test("ignores non-event frames without invoking onEvent", async () => {
250+
const { events } = await openClient();
251+
const ws = wsAt(0);
252+
253+
ws.message(JSON.stringify({ type: "pong" }));
254+
ws.message("not json");
255+
expect(events).toHaveLength(0);
256+
});
257+
});

packages/cli-core/src/commands/webhooks/replay.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ describe("webhooks replay", () => {
6767
label: "--until without --since",
6868
options: { msgId: "msg_1", until: "2026-05-01T00:00:00Z" },
6969
},
70+
{
71+
label: "--until alone (no <msg_id>, no --since)",
72+
options: { until: "2026-05-01T00:00:00Z" },
73+
},
7074
{
7175
label: "--since without --endpoint",
7276
options: { since: "2026-05-01T00:00:00Z" },

packages/cli-core/src/commands/webhooks/shared.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,17 @@ export function formatEndpointDetails(endpoint: WebhookEndpoint): void {
117117
}
118118
}
119119

120-
/** Split a comma-separated flag value into trimmed, non-empty entries. */
120+
/**
121+
* Split a comma-separated flag value into trimmed, non-empty entries. Returns
122+
* undefined when the flag was absent OR carried no real values (`""`, `","`,
123+
* whitespace) — so callers can treat an empty value as "not provided" instead
124+
* of sending an empty array.
125+
*/
121126
export function splitCommaList(value: string | undefined): string[] | undefined {
122127
if (value === undefined) return undefined;
123128
const parts = value
124129
.split(",")
125130
.map((part) => part.trim())
126131
.filter(Boolean);
127-
return parts;
132+
return parts.length > 0 ? parts : undefined;
128133
}

packages/cli-core/src/commands/webhooks/update.test.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,16 @@ describe("webhooks update", () => {
105105
expect(mockUpdateWebhookEndpoint).not.toHaveBeenCalled();
106106
});
107107

108+
test.each([
109+
{ label: "--events", options: { events: "" } },
110+
{ label: "--channels", options: { channels: " , " } },
111+
])("an empty $label value does not bypass the no-flags guard", async ({ options }) => {
112+
await expect(webhooksUpdate({ endpointId: "ep_1", ...options })).rejects.toMatchObject({
113+
code: ERROR_CODE.USAGE_ERROR,
114+
});
115+
expect(mockUpdateWebhookEndpoint).not.toHaveBeenCalled();
116+
});
117+
108118
test("prints the updated endpoint in human mode", async () => {
109119
await webhooksUpdate({ endpointId: "ep_1", description: "Updated" });
110120

packages/cli-core/src/commands/webhooks/verify.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,19 @@ describe("webhooks verify command", () => {
234234
).rejects.toThrow("in the past");
235235
});
236236

237+
test("mismatch on a future timestamp includes a humanized skew hint", async () => {
238+
const futureTimestamp = String(Number(TIMESTAMP) + 3600);
239+
const payloadPath = await writeTempFile("body.json", PAYLOAD);
240+
241+
await expect(
242+
webhooksVerify({
243+
...explicitFlags(),
244+
timestamp: futureTimestamp,
245+
payload: `@${payloadPath}`,
246+
}),
247+
).rejects.toThrow("in the future");
248+
});
249+
237250
test.each([
238251
{ label: "missing --secret", options: {} },
239252
{ label: "malformed --secret", options: { secret: "sk_nope" } },

0 commit comments

Comments
 (0)