Skip to content

Commit ca438d3

Browse files
hnbdrhnbdr
committed
test: kill remaining browser bridge mutation survivors with focused tests and narrow directives
1 parent ad9f1ab commit ca438d3

6 files changed

Lines changed: 461 additions & 2 deletions

File tree

‎src/core/webview/__tests__/ClineProvider.spec.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,130 @@ describe("ClineProvider", () => {
686686
})
687687
})
688688

689+
describe("resolveWebviewView html source selection", () => {
690+
const originalProbeSetting = process.env.ROO_CODE_THEME_FIXTURE_PROBE
691+
692+
function providerWithMode(extensionMode: number): ClineProvider {
693+
const context = { ...mockContext, extensionMode } as unknown as vscode.ExtensionContext
694+
return new ClineProvider(context, mockOutputChannel, "sidebar", new ContextProxy(context))
695+
}
696+
697+
afterEach(() => {
698+
if (originalProbeSetting === undefined) {
699+
delete process.env.ROO_CODE_THEME_FIXTURE_PROBE
700+
} else {
701+
process.env.ROO_CODE_THEME_FIXTURE_PROBE = originalProbeSetting
702+
}
703+
})
704+
705+
test("development mode without the probe flag serves the HMR html", async () => {
706+
delete process.env.ROO_CODE_THEME_FIXTURE_PROBE
707+
provider = providerWithMode(vscode.ExtensionMode.Development)
708+
const hmrSpy = vi.fn().mockResolvedValue("<!DOCTYPE html><title>hmr</title>")
709+
const htmlSpy = vi.fn().mockResolvedValue("<!DOCTYPE html><title>dist</title>")
710+
provider["getHMRHtmlContent"] = hmrSpy
711+
provider["getHtmlContent"] = htmlSpy
712+
713+
await provider.resolveWebviewView(mockWebviewView)
714+
715+
expect(hmrSpy).toHaveBeenCalledWith(mockWebviewView.webview)
716+
expect(htmlSpy).not.toHaveBeenCalled()
717+
expect(mockWebviewView.webview.html).toContain("hmr")
718+
})
719+
720+
test("development mode with the theme fixture probe serves the built html", async () => {
721+
process.env.ROO_CODE_THEME_FIXTURE_PROBE = "1"
722+
provider = providerWithMode(vscode.ExtensionMode.Development)
723+
const hmrSpy = vi.fn().mockResolvedValue("<!DOCTYPE html><title>hmr</title>")
724+
const htmlSpy = vi.fn().mockResolvedValue("<!DOCTYPE html><title>dist</title>")
725+
provider["getHMRHtmlContent"] = hmrSpy
726+
provider["getHtmlContent"] = htmlSpy
727+
728+
await provider.resolveWebviewView(mockWebviewView)
729+
730+
expect(htmlSpy).toHaveBeenCalledWith(mockWebviewView.webview)
731+
expect(hmrSpy).not.toHaveBeenCalled()
732+
expect(mockWebviewView.webview.html).toContain("dist")
733+
})
734+
735+
test("production mode serves the built html even without the probe flag", async () => {
736+
delete process.env.ROO_CODE_THEME_FIXTURE_PROBE
737+
provider = providerWithMode(vscode.ExtensionMode.Production)
738+
const hmrSpy = vi.fn().mockResolvedValue("<!DOCTYPE html><title>hmr</title>")
739+
const htmlSpy = vi.fn().mockResolvedValue("<!DOCTYPE html><title>dist</title>")
740+
provider["getHMRHtmlContent"] = hmrSpy
741+
provider["getHtmlContent"] = htmlSpy
742+
743+
await provider.resolveWebviewView(mockWebviewView)
744+
745+
expect(htmlSpy).toHaveBeenCalledWith(mockWebviewView.webview)
746+
expect(hmrSpy).not.toHaveBeenCalled()
747+
expect(mockWebviewView.webview.html).toContain("dist")
748+
})
749+
})
750+
751+
describe("convertToWebviewUri", () => {
752+
const waitForBridge = async () => {
753+
const started = Date.now()
754+
while (!BrowserBridgeServer.active(provider)) {
755+
if (Date.now() - started > 5_000) {
756+
throw new Error("Bridge did not become active")
757+
}
758+
await new Promise((resolve) => setTimeout(resolve, 10))
759+
}
760+
}
761+
762+
const fakeFileUri = { toString: () => "file:///test/asset.png" }
763+
764+
afterEach(() => {
765+
BrowserBridgeServer.disposeFor(provider)
766+
})
767+
768+
test("uses the virtual webview when the browser bridge is active", async () => {
769+
;(vscode.Uri.file as ReturnType<typeof vi.fn>).mockReturnValue(fakeFileUri)
770+
BrowserBridgeServer.enable(provider)
771+
await waitForBridge()
772+
// No real view resolved: only the bridge's virtual webview is available.
773+
provider["view"] = undefined
774+
const webview = BrowserBridgeServer.webviewFor(provider)!
775+
const uriSpy = vi.spyOn(webview, "asWebviewUri")
776+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
777+
778+
expect(provider.convertToWebviewUri("/test/asset.png")).toBe("file:///test/asset.png")
779+
780+
expect(uriSpy).toHaveBeenCalledWith(fakeFileUri)
781+
expect(errorSpy).not.toHaveBeenCalled()
782+
errorSpy.mockRestore()
783+
})
784+
785+
test("uses the resolved real webview when no bridge is active", async () => {
786+
;(vscode.Uri.file as ReturnType<typeof vi.fn>).mockReturnValue(fakeFileUri)
787+
provider["view"] = mockWebviewView
788+
const converted = { toString: () => "vscode-webview://converted" }
789+
mockWebviewView.webview.asWebviewUri.mockReturnValue(converted)
790+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
791+
792+
expect(provider.convertToWebviewUri("/test/asset.png")).toBe("vscode-webview://converted")
793+
794+
expect(mockWebviewView.webview.asWebviewUri).toHaveBeenCalledWith(fakeFileUri)
795+
expect(errorSpy).not.toHaveBeenCalled()
796+
errorSpy.mockRestore()
797+
})
798+
799+
test("logs the no-webview error and falls back to the file URI", () => {
800+
;(vscode.Uri.file as ReturnType<typeof vi.fn>).mockReturnValue(fakeFileUri)
801+
provider["view"] = undefined
802+
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
803+
804+
expect(provider.convertToWebviewUri("/test/asset.png")).toBe("file:///test/asset.png")
805+
806+
// The exact message proves the intended no-webview branch ran
807+
// (any thrown-error path would log the generic conversion failure).
808+
expect(errorSpy).toHaveBeenCalledWith("No webview available for URI conversion")
809+
errorSpy.mockRestore()
810+
})
811+
})
812+
689813
describe("logWebviewHiddenDiagnostics", () => {
690814
let visibilityCallback: () => void
691815

‎src/core/webview/__tests__/browserBridge.spec.ts‎

Lines changed: 227 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import type { ExtensionMessage, WebviewMessage } from "@roo-code/types"
2222

2323
import { allowNetConnect } from "../../../vitest.setup"
2424
import type { BridgeHost } from "../browserBridge"
25-
import { BrowserBridgeServer, getBrowserBridgePort } from "../browserBridge"
25+
import { BrowserBridgeServer, getBoundPort, getBrowserBridgePort } from "../browserBridge"
2626

2727
// The shared src/__mocks__/vscode.js lacks the ExtensionMode/env/commands/
2828
// window surface the bridge touches at runtime, so this spec supplies its own
@@ -43,6 +43,33 @@ vi.mock("vscode", () => ({
4343
// transport use the same host).
4444
allowNetConnect(/^127\.0\.0\.1(?::\d+)?$/)
4545

46+
// Capture the Server constructor options the bridge passes through the lazy
47+
// `import("socket.io")` in BrowserBridgeServer.start, without altering the
48+
// real server behavior (the round-trip tests still run against socket.io).
49+
type CapturedServerOptions = {
50+
cors: { origin: RegExp[] }
51+
transports: string[]
52+
}
53+
54+
const { socketIoOptions } = vi.hoisted(() => ({
55+
socketIoOptions: { current: undefined as CapturedServerOptions | undefined },
56+
}))
57+
58+
vi.mock("socket.io", async (importOriginal) => {
59+
const actual = await importOriginal<typeof import("socket.io")>()
60+
return {
61+
...actual,
62+
Server: class CapturingServer extends actual.Server {
63+
constructor(...args: ConstructorParameters<typeof actual.Server>) {
64+
super(...args)
65+
// The bridge always constructs `new Server(httpServer, options)`.
66+
const options = args[1] ?? args[0]
67+
socketIoOptions.current = options as CapturedServerOptions | undefined
68+
}
69+
},
70+
}
71+
})
72+
4673
function connectToBridge(port: number): Promise<Socket> {
4774
return new Promise((resolve, reject) => {
4875
const socket = io(`http://127.0.0.1:${port}`, {
@@ -114,6 +141,7 @@ describe("getBrowserBridgePort", () => {
114141
["float", "1.5", 0],
115142
["zero", "0", 0],
116143
["negative", "-1", 0],
144+
["upper boundary", "65536", 0],
117145
["out of range", "99999", 0],
118146
])("%s -> %i", (_label, value, expected) => {
119147
if (value === undefined) {
@@ -125,6 +153,70 @@ describe("getBrowserBridgePort", () => {
125153
})
126154
})
127155

156+
describe("getBoundPort", () => {
157+
it("returns the OS-assigned port for an AddressInfo object", () => {
158+
const httpServer = { address: () => ({ port: 43210, address: "127.0.0.1", family: "IPv4" }) }
159+
expect(getBoundPort(httpServer as never, 0)).toBe(43210)
160+
})
161+
162+
it("falls back to the requested port for a string (pipe) address", () => {
163+
const httpServer = { address: () => "\\\\.\\pipe\\bridge" }
164+
expect(getBoundPort(httpServer as never, 8080)).toBe(8080)
165+
})
166+
167+
it("falls back to the requested port while unbound (null address)", () => {
168+
const httpServer = { address: () => null }
169+
expect(getBoundPort(httpServer as never, 8080)).toBe(8080)
170+
})
171+
})
172+
173+
describe("BrowserBridgeServer.start (socket.io options)", () => {
174+
beforeEach(() => {
175+
socketIoOptions.current = undefined
176+
})
177+
178+
function captureOptions(): CapturedServerOptions {
179+
const options = socketIoOptions.current
180+
expect(options).toBeDefined()
181+
return options!
182+
}
183+
184+
it("passes local-origin CORS and the websocket+polling transports", async () => {
185+
const bridge = await startBridge()
186+
try {
187+
const options = captureOptions()
188+
expect(options.cors).toEqual({ origin: [expect.any(RegExp)] })
189+
expect(options.transports).toEqual(["websocket", "polling"])
190+
} finally {
191+
bridge["dispose"]()
192+
}
193+
})
194+
195+
it("restricts the CORS origin regex to bare localhost/loopback URLs", async () => {
196+
const bridge = await startBridge()
197+
try {
198+
const origin = captureOptions().cors.origin[0]
199+
const allowed = ["http://localhost", "http://127.0.0.1", "http://localhost:5173", "http://127.0.0.1:65535"]
200+
const denied = [
201+
"http://localhost:5173/path",
202+
"xhttp://localhost",
203+
"http://evil.com",
204+
"http://localhost:abc",
205+
"http://localhost:5173x",
206+
"https://localhost",
207+
]
208+
for (const value of allowed) {
209+
expect(origin.test(value)).toBe(true)
210+
}
211+
for (const value of denied) {
212+
expect(origin.test(value)).toBe(false)
213+
}
214+
} finally {
215+
bridge["dispose"]()
216+
}
217+
})
218+
})
219+
128220
describe("BrowserBridgeServer statics (WeakMap registry)", () => {
129221
const hosts: HostStub[] = []
130222
const sockets: Socket[] = []
@@ -303,6 +395,140 @@ describe("BrowserBridgeServer statics (WeakMap registry)", () => {
303395
const broadcast = await waitFor(() => inbound)
304396
expect(broadcast).toEqual(extensionMessage)
305397
})
398+
399+
it("disposeFor disposes the bridge server before dropping the entry", async () => {
400+
const host = createHost()
401+
hosts.push(host)
402+
403+
BrowserBridgeServer.enable(host)
404+
await waitFor(() => (BrowserBridgeServer.active(host) ? true : undefined))
405+
const bridge = BrowserBridgeServer["bridges"].get(host)!
406+
407+
// Wrap the private dispose so the real server still closes (no leaked port).
408+
const originalDispose = bridge["dispose"].bind(bridge)
409+
const disposeSpy = vi.fn(() => originalDispose())
410+
bridge["dispose"] = disposeSpy
411+
412+
BrowserBridgeServer.disposeFor(host)
413+
414+
expect(disposeSpy).toHaveBeenCalledTimes(1)
415+
expect(BrowserBridgeServer.active(host)).toBe(false)
416+
})
417+
418+
it("the virtual webview mirrors the vscode.Webview contract", async () => {
419+
const host = createHost()
420+
hosts.push(host)
421+
BrowserBridgeServer.enable(host)
422+
await waitFor(() => (BrowserBridgeServer.active(host) ? true : undefined))
423+
424+
const webview = BrowserBridgeServer.webviewFor(host)!
425+
expect(webview.options).toEqual({ enableScripts: true })
426+
expect(webview.cspSource).toBe("vscode-webview://bridge")
427+
expect(webview.html).toBe("")
428+
await expect(webview.postMessage({ type: "action", action: "chatButtonClicked" })).resolves.toBe(true)
429+
const uri = { toString: () => "file:///test/asset.png" } as never
430+
expect(webview.asWebviewUri(uri)).toBe(uri)
431+
})
432+
433+
it("an onWebviewMessage subscription stops delivering after dispose", async () => {
434+
const host = createHost()
435+
hosts.push(host)
436+
BrowserBridgeServer.enable(host)
437+
await waitFor(() => (BrowserBridgeServer.active(host) ? true : undefined))
438+
const bridge = BrowserBridgeServer["bridges"].get(host)!
439+
440+
const received: WebviewMessage[] = []
441+
const disposable = bridge["onWebviewMessage"]((message) => {
442+
received.push(message)
443+
})
444+
expect(typeof disposable.dispose).toBe("function")
445+
446+
const client = await connectToBridge(bridge["_port"])
447+
sockets.push(client)
448+
449+
const first: WebviewMessage = { type: "clearTask" }
450+
client.emit("webviewMessage", first)
451+
await waitFor(() => (received.length > 0 ? received[0] : undefined))
452+
453+
disposable.dispose()
454+
client.emit("webviewMessage", { type: "acceptInput" })
455+
await new Promise((resolve) => setTimeout(resolve, 150))
456+
expect(received).toEqual([first])
457+
})
458+
})
459+
460+
describe("BrowserBridgeServer occupied-port failure paths", () => {
461+
const originalPort = process.env.ROO_BROWSER_BRIDGE_PORT
462+
let blocker: ReturnType<typeof createServer>
463+
let occupied: number
464+
465+
beforeEach(async () => {
466+
// Occupy a real port and force the bridge to request exactly that one,
467+
// so start() deterministically fails with EADDRINUSE.
468+
blocker = createServer()
469+
await new Promise<void>((resolve) => blocker.listen(0, "127.0.0.1", resolve))
470+
occupied = (blocker.address() as AddressInfo).port
471+
process.env.ROO_BROWSER_BRIDGE_PORT = String(occupied)
472+
})
473+
474+
afterEach(async () => {
475+
if (originalPort === undefined) {
476+
delete process.env.ROO_BROWSER_BRIDGE_PORT
477+
} else {
478+
process.env.ROO_BROWSER_BRIDGE_PORT = originalPort
479+
}
480+
await new Promise<void>((resolve) => blocker.close(() => resolve()))
481+
})
482+
483+
it("start without onError resolves undefined instead of rejecting", async () => {
484+
await expect(BrowserBridgeServer["start"](() => {})).resolves.toBeUndefined()
485+
})
486+
487+
it("enable leaves the host inert when the bridge fails to start", async () => {
488+
const host = createHost()
489+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {})
490+
try {
491+
BrowserBridgeServer.enable(host)
492+
await waitFor(() =>
493+
logSpy.mock.calls.some(([message]) => String(message).includes("[BrowserBridge] Failed to start"))
494+
? true
495+
: undefined,
496+
)
497+
// Grace for the awaited server.close() and any (mutant) bind attempt.
498+
await new Promise((resolve) => setTimeout(resolve, 100))
499+
500+
expect(BrowserBridgeServer.active(host)).toBe(false)
501+
expect(host.setWebviewMessageListener).not.toHaveBeenCalled()
502+
} finally {
503+
logSpy.mockRestore()
504+
BrowserBridgeServer.disposeFor(host)
505+
}
506+
})
507+
508+
it("a second enable never starts a second listening server", async () => {
509+
const host = createHost()
510+
// Port 0 (not the occupied override) so the second start *can* succeed:
511+
// without the bridges.has guard its "[BrowserBridge] Listening" log is
512+
// the observable proof a second server bound.
513+
delete process.env.ROO_BROWSER_BRIDGE_PORT
514+
const logSpy = vi.spyOn(console, "log").mockImplementation(() => {})
515+
try {
516+
BrowserBridgeServer.enable(host)
517+
await waitFor(() => (BrowserBridgeServer.active(host) ? true : undefined))
518+
519+
BrowserBridgeServer.enable(host)
520+
// Grace for a would-be second server to bind and log.
521+
await new Promise((resolve) => setTimeout(resolve, 250))
522+
523+
const listening = logSpy.mock.calls.filter(([message]) =>
524+
String(message).includes("[BrowserBridge] Listening"),
525+
)
526+
expect(listening).toHaveLength(1)
527+
} finally {
528+
logSpy.mockRestore()
529+
BrowserBridgeServer.disposeFor(host)
530+
}
531+
})
306532
})
307533

308534
describe("BrowserBridgeServer.registerCommand (dev-only self-gating)", () => {

0 commit comments

Comments
 (0)