refactor(coding-agent): migrated fetch mocks to hookFetch utility with resource cleanup

- Replaced vi.spyOn fetch mocking pattern with hookFetch utility across 5 test files.
- Migrated fetch mocks to use resource management with 'using' keyword for automatic cleanup.
- Removed @ts-expect-error comments related to fetch.preconnect type issues.
- Updated ts-hook-fetch rule documentation with expanded patterns and clearer lifecycle guidance.

.omp/rules/ts-hook-fetch.md

@@ -1,20 +1,20 @@
 ---
-description: Use hookFetch instead of assigning globalThis.fetch directly in tests
-condition: "globalThis\\.fetch\\s*="
+description: Use hookFetch instead of assigning or spying on globalThis.fetch in tests
+condition: "globalThis\\.fetch\\s*=|spyOn\\(globalThis.*fetch"
 scope: "tool:edit(**/*.test.{ts,tsx,js,jsx}), tool:write(**/*.test.{ts,tsx,js,jsx})"
 ---
 
-**Do not assign `globalThis.fetch = ...` directly in tests.**
+**Do not assign `globalThis.fetch` or use `vi.spyOn(globalThis, "fetch")` in tests.**
 
 ## Why it's wrong
 
-- It bypasses the project's standard fetch mocking helper
-- It is easier to forget restoration and leak state across tests
-- It makes test mocking inconsistent across the codebase
+- Forgetting restoration leaks state across tests
+- `vi.spyOn` ties fetch mocking to vitest lifecycle instead of explicit scoping
+- Makes test mocking inconsistent across the codebase
 
 ## What to use instead
 
-Use `hookFetch` from `@oh-my-pi/pi-utils`:
+Use `hookFetch` from `@oh-my-pi/pi-utils`. It returns a `Disposable` — use `using` for automatic cleanup:
 
 ```ts
 import { hookFetch } from "@oh-my-pi/pi-utils";
@@ -30,8 +30,22 @@ using _hook = hookFetch((input, init, next) => {
 // WRONG
 globalThis.fetch = async () => new Response("ok");
 
-// RIGHT
-using _hook = hookFetch(() => new Response("ok"));
-```
+// WRONG
+vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("ok"));
 
-If you need to intercept fetch in tests, use `hookFetch`.
+// RIGHT — fixed response
+using _hook = hookFetch(() => new Response("ok"));
+
+// RIGHT — conditional mock with passthrough
+using _hook = hookFetch((input, init, next) => {
+	if (String(input).includes("127.0.0.1")) {
+		return new Response(JSON.stringify({ data: [] }));
+	}
+	return next(input, init);
+});
+
+// RIGHT — when you need vi.fn() for mock assertions
+const fetchSpy = vi.fn(() => new Response("ok"));
+using _hook = hookFetch(fetchSpy);
+// later: expect(fetchSpy.mock.calls[0]).toEqual(...)
+```

packages/coding-agent/test/compaction.test.ts

@@ -3,6 +3,7 @@ import * as path from "node:path";
 import type { AgentMessage } from "@oh-my-pi/pi-agent-core";
 import { getBundledModel } from "@oh-my-pi/pi-ai/models";
 import type { AssistantMessage, Model, Usage } from "@oh-my-pi/pi-ai/types";
+import { hookFetch } from "@oh-my-pi/pi-utils";
 
 const completeSimpleMock = vi.fn();
 
@@ -318,12 +319,14 @@ describe("remote compaction setting", () => {
 			throw new Error("Expected compaction preparation");
 		}
 
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(JSON.stringify({ summary: "remote summary" }), {
-				status: 200,
-				headers: { "Content-Type": "application/json" },
-			}),
+		const fetchSpy = vi.fn(
+			() =>
+				new Response(JSON.stringify({ summary: "remote summary" }), {
+					status: 200,
+					headers: { "Content-Type": "application/json" },
+				}),
 		);
+		using _hook = hookFetch(fetchSpy);
 		const completeSpy = completeSimpleMock
 			.mockResolvedValueOnce(createAssistantMessage("Local history summary"))
 			.mockResolvedValueOnce(createAssistantMessage("Local turn summary"))
@@ -394,12 +397,14 @@ describe("remote compaction setting", () => {
 			{ type: "message", role: "user", content: [{ type: "input_text", text: "Compacted retained user" }] },
 			{ type: "compaction", encrypted_content: "new_encrypted" },
 		];
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(JSON.stringify({ output: remoteOutput }), {
-				status: 200,
-				headers: { "Content-Type": "application/json" },
-			}),
+		const fetchSpy = vi.fn(
+			() =>
+				new Response(JSON.stringify({ output: remoteOutput }), {
+					status: 200,
+					headers: { "Content-Type": "application/json" },
+				}),
 		);
+		using _hook = hookFetch(fetchSpy);
 		completeSimpleMock
 			.mockResolvedValueOnce(createAssistantMessage("History summary"))
 			.mockResolvedValueOnce(createAssistantMessage("Turn prefix summary"))
@@ -458,12 +463,14 @@ describe("remote compaction setting", () => {
 		});
 		if (!preparation) throw new Error("Expected compaction preparation");
 
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(JSON.stringify({ output: [{ type: "compaction", encrypted_content: "new_encrypted" }] }), {
-				status: 200,
-				headers: { "Content-Type": "application/json" },
-			}),
+		const fetchSpy = vi.fn(
+			() =>
+				new Response(JSON.stringify({ output: [{ type: "compaction", encrypted_content: "new_encrypted" }] }), {
+					status: 200,
+					headers: { "Content-Type": "application/json" },
+				}),
 		);
+		using _hook = hookFetch(fetchSpy);
 		completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary"));
 
 		await compact(preparation, model, "test-api-key");
@@ -499,12 +506,14 @@ describe("remote compaction setting", () => {
 		});
 		if (!preparation) throw new Error("Expected compaction preparation");
 
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(JSON.stringify({ output: [{ type: "compaction", encrypted_content: "new_encrypted" }] }), {
-				status: 200,
-				headers: { "Content-Type": "application/json" },
-			}),
+		const fetchSpy = vi.fn(
+			() =>
+				new Response(JSON.stringify({ output: [{ type: "compaction", encrypted_content: "new_encrypted" }] }), {
+					status: 200,
+					headers: { "Content-Type": "application/json" },
+				}),
 		);
+		using _hook = hookFetch(fetchSpy);
 		completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary"));
 
 		await compact(preparation, model, "test-api-key");
@@ -541,12 +550,14 @@ describe("remote compaction setting", () => {
 			{ type: "message", role: "assistant", content: [{ type: "output_text", text: "Kept assistant" }] },
 			{ type: "compaction", encrypted_content: "new_encrypted" },
 		];
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(JSON.stringify({ output: remoteOutput }), {
-				status: 200,
-				headers: { "Content-Type": "application/json" },
-			}),
+		const fetchSpy = vi.fn(
+			() =>
+				new Response(JSON.stringify({ output: remoteOutput }), {
+					status: 200,
+					headers: { "Content-Type": "application/json" },
+				}),
 		);
+		using _hook = hookFetch(fetchSpy);
 		completeSimpleMock.mockResolvedValue(createAssistantMessage("Short summary"));
 
 		const result = await compact(preparation, model, "test-api-key", undefined, undefined, {

packages/coding-agent/test/tools/fetch-kagi-toggle.test.ts

@@ -12,7 +12,7 @@ import type { LoadPageResult } from "@oh-my-pi/pi-coding-agent/web/scrapers/type
 import * as scrapers from "@oh-my-pi/pi-coding-agent/web/scrapers/types";
 import * as scraperUtils from "@oh-my-pi/pi-coding-agent/web/scrapers/utils";
 import * as natives from "@oh-my-pi/pi-natives";
-import { ptree, Snowflake } from "@oh-my-pi/pi-utils";
+import { hookFetch, ptree, Snowflake } from "@oh-my-pi/pi-utils";
 
 describe("fetch tool Kagi summarization toggle", () => {
 	let testDir: string;
@@ -73,7 +73,7 @@ describe("fetch tool Kagi summarization toggle", () => {
 		const loadPageSpy = mockLoadPage();
 		const summarizeSpy = vi.spyOn(kagi, "summarizeUrlWithKagi").mockResolvedValue("x".repeat(150));
 		vi.spyOn(toolsManager, "ensureTool").mockResolvedValue(undefined);
-		vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("blocked", { status: 500, statusText: "Blocked" }));
+		using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" }));
 
 		const result = await tool.execute("fetch-1", { url: "https://example.com" });
 
@@ -88,7 +88,7 @@ describe("fetch tool Kagi summarization toggle", () => {
 		const loadPageSpy = mockLoadPage();
 		const summarizeSpy = vi.spyOn(kagi, "summarizeUrlWithKagi").mockResolvedValue("x".repeat(150));
 		vi.spyOn(toolsManager, "ensureTool").mockResolvedValue(undefined);
-		vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("blocked", { status: 500, statusText: "Blocked" }));
+		using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" }));
 
 		const result = await tool.execute("fetch-2", { url: "https://example.com" });
 
@@ -425,9 +425,7 @@ describe("fetch tool Kagi summarization toggle", () => {
 					content: "",
 				};
 			});
-			vi.spyOn(globalThis, "fetch").mockResolvedValue(
-				new Response("blocked", { status: 500, statusText: "Blocked" }),
-			);
+			using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" }));
 			vi.spyOn(toolsManager, "ensureTool").mockResolvedValue(undefined);
 			vi.spyOn(natives, "htmlToMarkdown").mockResolvedValue(renderedMarkdown);
 
@@ -513,9 +511,7 @@ describe("fetch tool Kagi summarization toggle", () => {
 					content: "",
 				};
 			});
-			vi.spyOn(globalThis, "fetch").mockResolvedValue(
-				new Response("blocked", { status: 500, statusText: "Blocked" }),
-			);
+			using _hook = hookFetch(() => new Response("blocked", { status: 500, statusText: "Blocked" }));
 			vi.spyOn(toolsManager, "ensureTool").mockResolvedValue("/usr/bin/trafilatura");
 
 			const result = await tool.execute("fetch-section-llms", { url: pageUrl });

packages/coding-agent/test/tools/web-search-exa.test.ts

@@ -1,4 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
+import { hookFetch } from "@oh-my-pi/pi-utils";
 import { runSearchQuery } from "../../src/web/search";
 import {
 	buildExaRequestBody,
@@ -228,9 +229,8 @@ describe("searchExa", () => {
 		delete process.env.EXA_API_KEY;
 	});
 
-	function mockFetch(responseBody: unknown, status = 200) {
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async (_url: string | URL | Request, init?: RequestInit) => {
+	function mockFetch(responseBody: unknown, status = 200): Disposable {
+		return hookFetch((_url, init) => {
 			if (init?.body) {
 				capturedRequestBody = JSON.parse(init.body as string);
 			}
@@ -242,7 +242,7 @@ describe("searchExa", () => {
 	}
 
 	it("populates answer from per-result summaries", async () => {
-		mockFetch(makeMockExaResponse());
+		using _hook = mockFetch(makeMockExaResponse());
 		const result = await searchExa({ query: "test query" });
 		expect(result.provider).toBe("exa");
 		expect(result.answer).toBeDefined();
@@ -253,7 +253,7 @@ describe("searchExa", () => {
 	});
 
 	it("returns answer=undefined when no summaries are present", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({ results: [{ title: "No Summary", url: "https://nosummary.com", text: "some text" }] }),
 		);
 		const result = await searchExa({ query: "no answer query" });
@@ -263,28 +263,28 @@ describe("searchExa", () => {
 	});
 
 	it("returns answer=undefined when results array is empty", async () => {
-		mockFetch(makeMockExaResponse({ results: [] }));
+		using _hook = mockFetch(makeMockExaResponse({ results: [] }));
 		const result = await searchExa({ query: "empty" });
 		expect(result.answer).toBeUndefined();
 		expect(result.sources).toHaveLength(0);
 	});
 
 	it("returns answer=undefined when results is missing from response", async () => {
-		mockFetch({ requestId: "req-empty" });
+		using _hook = mockFetch({ requestId: "req-empty" });
 		const result = await searchExa({ query: "nothing" });
 		expect(result.answer).toBeUndefined();
 		expect(result.sources).toHaveLength(0);
 	});
 
 	it("sends contents.summary in request body", async () => {
-		mockFetch(makeMockExaResponse());
+		using _hook = mockFetch(makeMockExaResponse());
 		await searchExa({ query: "check body" });
 		expect(capturedRequestBody).toBeDefined();
 		expect(capturedRequestBody!.contents).toEqual({ summary: { query: "check body" } });
 	});
 
 	it("sends correct full request shape", async () => {
-		mockFetch(makeMockExaResponse());
+		using _hook = mockFetch(makeMockExaResponse());
 		await searchExa({ query: "shape test", num_results: 5, type: "neural" });
 		expect(capturedRequestBody).toEqual({
 			query: "shape test",
@@ -295,7 +295,7 @@ describe("searchExa", () => {
 	});
 
 	it("prefers summary over text for snippet field", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({
 				results: [{ title: "Has Both", url: "https://both.com", text: "full text here", summary: "summary here" }],
 			}),
@@ -305,7 +305,7 @@ describe("searchExa", () => {
 	});
 
 	it("falls back to text when summary is null", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({
 				results: [{ title: "Text Only", url: "https://text.com", text: "fallback text", summary: null }],
 			}),
@@ -315,7 +315,7 @@ describe("searchExa", () => {
 	});
 
 	it("falls back to highlights when both summary and text are null", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({
 				results: [
 					{
@@ -333,7 +333,7 @@ describe("searchExa", () => {
 	});
 
 	it("skips results without url", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({
 				results: [
 					{ title: "No URL", url: null, summary: "orphan" },
@@ -347,7 +347,7 @@ describe("searchExa", () => {
 	});
 
 	it("falls back to text when summary is empty string (not just null)", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({
 				results: [{ title: "Empty Summary", url: "https://empty.com", text: "real text", summary: "" }],
 			}),
@@ -357,7 +357,7 @@ describe("searchExa", () => {
 	});
 
 	it("does not include url-less results in synthesized answer", async () => {
-		mockFetch(
+		using _hook = mockFetch(
 			makeMockExaResponse({
 				results: [
 					{ title: "No URL", url: null, summary: "ghost summary" },
@@ -373,13 +373,13 @@ describe("searchExa", () => {
 
 	it("uses Exa MCP when API key is missing", async () => {
 		delete process.env.EXA_API_KEY;
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		const fetchSpy = vi.fn(async () => {
 			return new Response(JSON.stringify({ jsonrpc: "2.0", id: "mcp-1", result: makeMockExaResponse() }), {
 				status: 200,
 				headers: { "Content-Type": "application/json" },
 			});
 		});
+		using _hook = hookFetch(fetchSpy);
 
 		const result = await searchExa({ query: "no key" });
 		expect(result.provider).toBe("exa");
@@ -393,8 +393,7 @@ describe("searchExa", () => {
 
 	it("accepts MCP structuredContent search payloads when API key is missing", async () => {
 		delete process.env.EXA_API_KEY;
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		using _hook = hookFetch(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -414,8 +413,7 @@ describe("searchExa", () => {
 	it("accepts MCP text content JSON payloads when API key is missing", async () => {
 		delete process.env.EXA_API_KEY;
 		const payload = makeMockExaResponse();
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		const fetchSpy = vi.fn(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -427,6 +425,7 @@ describe("searchExa", () => {
 				{ status: 200, headers: { "Content-Type": "application/json" } },
 			);
 		});
+		using _hook = hookFetch(fetchSpy);
 
 		const result = await searchExa({ query: "content payload" });
 		expect(result.provider).toBe("exa");
@@ -450,8 +449,7 @@ describe("searchExa", () => {
 			"URL: https://plain-beta.com",
 			"Text: Beta snippet",
 		].join("\n");
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		const fetchSpy = vi.fn(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -461,6 +459,7 @@ describe("searchExa", () => {
 				{ status: 200, headers: { "Content-Type": "application/json" } },
 			);
 		});
+		using _hook = hookFetch(fetchSpy);
 
 		const result = await searchExa({ query: "plain text content payload" });
 		expect(result.provider).toBe("exa");
@@ -494,8 +493,7 @@ describe("searchExa", () => {
 			"URL: https://crlf-beta.com",
 			"Text: Second result",
 		].join("\r\n");
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		using _hook = hookFetch(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -526,8 +524,7 @@ describe("searchExa", () => {
 			"URL: https://plain-beta.com",
 			"Text: Beta snippet",
 		].join("\n");
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		using _hook = hookFetch(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -556,8 +553,7 @@ describe("searchExa", () => {
 			"URL: https://result-two.com",
 			"Text: Second plain-text result",
 		].join("\n");
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		using _hook = hookFetch(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -576,8 +572,7 @@ describe("searchExa", () => {
 
 	it("runSearchQuery with provider=exa succeeds without EXA_API_KEY for MCP structuredContent", async () => {
 		delete process.env.EXA_API_KEY;
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		using _hook = hookFetch(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -596,8 +591,7 @@ describe("searchExa", () => {
 
 	it("throws clear error when MCP content payload is not parseable JSON", async () => {
 		delete process.env.EXA_API_KEY;
-		// @ts-expect-error - test mock doesn't need fetch.preconnect
-		vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
+		using _hook = hookFetch(async () => {
 			return new Response(
 				JSON.stringify({
 					jsonrpc: "2.0",
@@ -616,7 +610,7 @@ describe("searchExa", () => {
 	});
 
 	it("throws SearchProviderError on non-ok HTTP response", async () => {
-		mockFetch("Forbidden", 403);
+		using _hook = mockFetch("Forbidden", 403);
 		await expect(searchExa({ query: "forbidden" })).rejects.toThrow("Exa API error (403)");
 	});
 });

packages/coding-agent/test/tools/web-search-kagi.test.ts

@@ -1,4 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
+import { hookFetch } from "@oh-my-pi/pi-utils";
 import { searchWithKagi } from "../../src/web/kagi";
 import { searchKagi } from "../../src/web/search/providers/kagi";
 import { SearchProviderError } from "../../src/web/search/types";
@@ -17,11 +18,12 @@ describe("Kagi web search error handling", () => {
 		const providerMessage =
 			"Kagi Search API is in beta. Please contact support@kagi.com to enable API access for your account.";
 
-		vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(JSON.stringify({ error: [{ code: 401, msg: providerMessage }] }), {
-				status: 401,
-				headers: { "Content-Type": "application/json" },
-			}),
+		using _hook = hookFetch(
+			() =>
+				new Response(JSON.stringify({ error: [{ code: 401, msg: providerMessage }] }), {
+					status: 401,
+					headers: { "Content-Type": "application/json" },
+				}),
 		);
 
 		try {
@@ -35,29 +37,30 @@ describe("Kagi web search error handling", () => {
 	});
 
 	it("falls back to plain text for non-JSON error bodies", async () => {
-		vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response("upstream unavailable", { status: 503 }));
+		using _hook = hookFetch(() => new Response("upstream unavailable", { status: 503 }));
 
 		await expect(searchWithKagi("plain text error")).rejects.toThrow("Kagi API error (503): upstream unavailable");
 	});
 
 	it("preserves successful search parsing", async () => {
-		vi.spyOn(globalThis, "fetch").mockResolvedValue(
-			new Response(
-				JSON.stringify({
-					meta: { id: "req-kagi-success" },
-					data: [
-						{
-							t: 0,
-							url: "https://example.com/article",
-							title: "Example Article",
-							snippet: "Example snippet",
-							published: "2025-01-01T00:00:00Z",
-						},
-						{ t: 1, list: ["What is Kagi Search API beta access?"] },
-					],
-				}),
-				{ status: 200, headers: { "Content-Type": "application/json" } },
-			),
+		using _hook = hookFetch(
+			() =>
+				new Response(
+					JSON.stringify({
+						meta: { id: "req-kagi-success" },
+						data: [
+							{
+								t: 0,
+								url: "https://example.com/article",
+								title: "Example Article",
+								snippet: "Example snippet",
+								published: "2025-01-01T00:00:00Z",
+							},
+							{ t: 1, list: ["What is Kagi Search API beta access?"] },
+						],
+					}),
+					{ status: 200, headers: { "Content-Type": "application/json" } },
+				),
 		);
 
 		await expect(searchWithKagi("success case")).resolves.toEqual({