Skip to content

Commit 2397a25

Browse files
committed
fix(clownfish): address review for ghcrawl-207035-agentic-merge (1)
1 parent e73b08c commit 2397a25

5 files changed

Lines changed: 167 additions & 3 deletions

File tree

src/gateway/server-ws-runtime.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
type GatewayWsSharedHandlerParams,
66
} from "./server/ws-connection.js";
77

8-
type GatewayWsRuntimeParams = GatewayWsSharedHandlerParams & {
8+
type GatewayWsRuntimeParams = Omit<GatewayWsSharedHandlerParams, "refreshHealthSnapshot"> & {
99
logGateway: ReturnType<typeof createSubsystemLogger>;
1010
logHealth: ReturnType<typeof createSubsystemLogger>;
1111
logWsControl: ReturnType<typeof createSubsystemLogger>;
@@ -37,6 +37,7 @@ export function attachGatewayWsHandlers(params: GatewayWsRuntimeParams) {
3737
browserRateLimiter: params.browserRateLimiter,
3838
gatewayMethods: params.gatewayMethods,
3939
events: params.events,
40+
refreshHealthSnapshot: params.context.refreshHealthSnapshot,
4041
logGateway: params.logGateway,
4142
logHealth: params.logHealth,
4243
logWsControl: params.logWsControl,

src/gateway/server/ws-connection.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ describe("attachGatewayWsConnectionHandler", () => {
7070
getResolvedAuth: () => currentAuth,
7171
gatewayMethods: [],
7272
events: [],
73+
refreshHealthSnapshot: vi.fn(async () => ({}) as never),
7374
logGateway: createLogger() as never,
7475
logHealth: createLogger() as never,
7576
logWsControl: createLogger() as never,

src/gateway/server/ws-connection.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ export type GatewayWsSharedHandlerParams = {
133133
browserRateLimiter?: AuthRateLimiter;
134134
gatewayMethods: string[];
135135
events: string[];
136+
refreshHealthSnapshot: GatewayRequestContext["refreshHealthSnapshot"];
136137
};
137138

138139
export type AttachGatewayWsConnectionHandlerParams = GatewayWsSharedHandlerParams & {
@@ -168,6 +169,7 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti
168169
browserRateLimiter,
169170
gatewayMethods,
170171
events,
172+
refreshHealthSnapshot,
171173
logGateway,
172174
logHealth,
173175
logWsControl,
@@ -402,6 +404,7 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti
402404
events,
403405
extraHandlers,
404406
buildRequestContext,
407+
refreshHealthSnapshot,
405408
send,
406409
close,
407410
isClosed: () => closed,
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import { EventEmitter } from "node:events";
2+
import type { IncomingMessage } from "node:http";
3+
import { beforeEach, describe, expect, it, vi } from "vitest";
4+
import type { WebSocket } from "ws";
5+
import type { ResolvedGatewayAuth } from "../../auth.js";
6+
import { PROTOCOL_VERSION } from "../../protocol/index.js";
7+
import type { GatewayRequestContext } from "../../server-methods/types.js";
8+
9+
const {
10+
buildGatewaySnapshotMock,
11+
getHealthCacheMock,
12+
getHealthVersionMock,
13+
incrementPresenceVersionMock,
14+
loadConfigMock,
15+
upsertPresenceMock,
16+
} = vi.hoisted(() => ({
17+
buildGatewaySnapshotMock: vi.fn(() => ({
18+
presence: [],
19+
health: {},
20+
stateVersion: { presence: 1, health: 1 },
21+
uptimeMs: 1,
22+
sessionDefaults: {
23+
defaultAgentId: "main",
24+
mainKey: "main",
25+
mainSessionKey: "main",
26+
scope: "per-sender",
27+
},
28+
})),
29+
getHealthCacheMock: vi.fn(() => null),
30+
getHealthVersionMock: vi.fn(() => 1),
31+
incrementPresenceVersionMock: vi.fn(() => 2),
32+
loadConfigMock: vi.fn(() => ({
33+
gateway: {
34+
auth: { mode: "none" },
35+
controlUi: {},
36+
},
37+
})),
38+
upsertPresenceMock: vi.fn(),
39+
}));
40+
41+
vi.mock("../../../config/config.js", () => ({
42+
loadConfig: loadConfigMock,
43+
}));
44+
45+
vi.mock("../../../infra/system-presence.js", () => ({
46+
upsertPresence: upsertPresenceMock,
47+
}));
48+
49+
vi.mock("../../server-methods.js", () => ({
50+
handleGatewayRequest: vi.fn(),
51+
}));
52+
53+
vi.mock("../health-state.js", () => ({
54+
buildGatewaySnapshot: buildGatewaySnapshotMock,
55+
getHealthCache: getHealthCacheMock,
56+
getHealthVersion: getHealthVersionMock,
57+
incrementPresenceVersion: incrementPresenceVersionMock,
58+
}));
59+
60+
import { attachGatewayWsMessageHandler } from "./message-handler.js";
61+
62+
function createLogger() {
63+
return {
64+
debug: vi.fn(),
65+
info: vi.fn(),
66+
warn: vi.fn(),
67+
error: vi.fn(),
68+
};
69+
}
70+
71+
describe("attachGatewayWsMessageHandler post-connect health refresh", () => {
72+
beforeEach(() => {
73+
vi.clearAllMocks();
74+
});
75+
76+
it("uses the injected runtime-aware health refresh after hello", async () => {
77+
let resolveRefresh: (() => void) | undefined;
78+
const refreshHealthSnapshot = vi.fn(
79+
() =>
80+
new Promise((resolve) => {
81+
resolveRefresh = () => resolve({} as never);
82+
}),
83+
) as GatewayRequestContext["refreshHealthSnapshot"];
84+
const socket = Object.assign(new EventEmitter(), {
85+
_receiver: {},
86+
send: vi.fn((_payload: string, cb?: (err?: Error) => void) => {
87+
cb?.();
88+
}),
89+
}) as unknown as WebSocket;
90+
let client: unknown = null;
91+
const resolvedAuth: ResolvedGatewayAuth = {
92+
mode: "token",
93+
token: "test-token",
94+
allowTailscale: false,
95+
};
96+
97+
attachGatewayWsMessageHandler({
98+
socket,
99+
upgradeReq: {
100+
headers: { host: "127.0.0.1:19001" },
101+
socket: { localAddress: "127.0.0.1", remoteAddress: "127.0.0.1" },
102+
} as unknown as IncomingMessage,
103+
connId: "conn-1",
104+
remoteAddr: "127.0.0.1",
105+
localAddr: "127.0.0.1",
106+
requestHost: "127.0.0.1:19001",
107+
connectNonce: "nonce-1",
108+
getResolvedAuth: () => resolvedAuth,
109+
gatewayMethods: [],
110+
events: [],
111+
extraHandlers: {},
112+
buildRequestContext: () => ({}) as GatewayRequestContext,
113+
refreshHealthSnapshot,
114+
send: vi.fn(),
115+
close: vi.fn(),
116+
isClosed: () => false,
117+
clearHandshakeTimer: vi.fn(),
118+
getClient: () => client as never,
119+
setClient: (next) => {
120+
client = next;
121+
},
122+
setHandshakeState: vi.fn(),
123+
setCloseCause: vi.fn(),
124+
setLastFrameMeta: vi.fn(),
125+
originCheckMetrics: { hostHeaderFallbackAccepted: 0 },
126+
logGateway: createLogger() as never,
127+
logHealth: createLogger() as never,
128+
logWsControl: createLogger() as never,
129+
});
130+
131+
socket.emit(
132+
"message",
133+
JSON.stringify({
134+
type: "req",
135+
id: "connect-1",
136+
method: "connect",
137+
params: {
138+
minProtocol: PROTOCOL_VERSION,
139+
maxProtocol: PROTOCOL_VERSION,
140+
client: {
141+
id: "test",
142+
version: "dev",
143+
platform: "test",
144+
mode: "test",
145+
},
146+
auth: { token: "test-token" },
147+
role: "operator",
148+
caps: [],
149+
},
150+
}),
151+
);
152+
153+
await vi.waitFor(() => {
154+
expect(refreshHealthSnapshot).toHaveBeenCalledWith({ probe: true });
155+
});
156+
resolveRefresh?.();
157+
});
158+
});

src/gateway/server/ws-connection/message-handler.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ import {
113113
getHealthCache,
114114
getHealthVersion,
115115
incrementPresenceVersion,
116-
refreshGatewayHealthSnapshot,
117116
} from "../health-state.js";
118117
import { resolveSharedGatewaySessionGeneration } from "../ws-shared-generation.js";
119118
import type { GatewayWsClient } from "../ws-types.js";
@@ -197,6 +196,7 @@ export function attachGatewayWsMessageHandler(params: {
197196
events: string[];
198197
extraHandlers: GatewayRequestHandlers;
199198
buildRequestContext: () => GatewayRequestContext;
199+
refreshHealthSnapshot: GatewayRequestContext["refreshHealthSnapshot"];
200200
send: (obj: unknown) => void;
201201
close: (code?: number, reason?: string) => void;
202202
isClosed: () => boolean;
@@ -235,6 +235,7 @@ export function attachGatewayWsMessageHandler(params: {
235235
events,
236236
extraHandlers,
237237
buildRequestContext,
238+
refreshHealthSnapshot,
238239
send,
239240
close,
240241
isClosed,
@@ -1479,7 +1480,7 @@ export function attachGatewayWsMessageHandler(params: {
14791480
presence: snapshot.presence.length,
14801481
stateVersion: snapshot.stateVersion.presence,
14811482
});
1482-
void refreshGatewayHealthSnapshot({ probe: true }).catch((err) =>
1483+
void refreshHealthSnapshot({ probe: true }).catch((err) =>
14831484
logHealth.error(`post-connect health refresh failed: ${formatError(err)}`),
14841485
);
14851486
return;

0 commit comments

Comments
 (0)