ca-marketplace-scraper/docs/superpowers/plans/2026-04-28-code-smell-cleanup.md

# Code Smell Cleanup Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Fix concrete code smells found in repo review without changing marketplace behavior or relaxing lint/type rules.

**Architecture:** Start with correctness bugs at transport boundaries, then remove secret-leaking query/log paths, then reduce duplicate parsing and HTTP code. Keep marketplace behavior inside `packages/core`, API routes thin, and MCP as JSON-RPC transport only.

**Tech Stack:** Bun `1.3.13`, TypeScript strict mode, `bun:test`, Biome, framework-free `Bun.serve` adapters.

---

## File Structure

- Modify: `packages/mcp-server/src/protocol/handler.ts`
  - Fix JSON-RPC notification detection.
  - Preserve numeric zero arguments.
  - Extract shared API call/query-param helpers.
  - Stop logging full URLs with cookie-bearing params.
- Modify: `packages/mcp-server/src/protocol/tools.ts`
  - Remove `cookies` from Kijiji MCP schema or mark it as unsupported after API route no longer accepts it.
- Modify: `packages/mcp-server/test/protocol.test.ts`
  - Add coverage for `id: 0`.
  - Add coverage for zero-valued numeric args.
  - Add coverage that no cookie input is exposed or forwarded.
- Modify: `packages/api-server/src/routes/kijiji.ts`
  - Stop accepting `cookies` query param.
  - Use strict integer parsing helper.
  - Use shared route helpers.
- Modify: `packages/api-server/src/routes/facebook.ts`
  - Use strict integer parsing helper.
  - Use shared route helpers.
- Modify: `packages/api-server/src/routes/ebay.ts`
  - Use strict integer parsing helper.
  - Use shared route helpers.
- Create: `packages/api-server/src/routes/helpers.ts`
  - Own shared route parsing and response helpers.
- Modify: `packages/api-server/test/routes.test.ts`
  - Update Kijiji cookie expectations.
  - Add strict-number validation tests.
- Modify: `packages/core/src/utils/http.ts`
  - Add options for deterministic retry jitter and optional response URL return.
  - Keep existing callers compatible.
- Modify: `packages/core/src/scrapers/facebook.ts`
  - Replace local `HttpError` and `fetchHtml` with shared utility.
  - Keep Facebook-specific headers/cookie behavior via options.
- Modify: `packages/core/src/scrapers/ebay.ts`
  - Replace direct `fetch`/local `HttpError` with shared utility.
  - Replace `console.error` with repo logger.
- Modify: `packages/core/src/scrapers/kijiji.ts`
  - Delete dead `_parseListing` if tests confirm no callers.
  - Replace direct `setTimeout` calls with `delay` helper.
  - Replace `console.error` with repo logger.
- Modify: `packages/core/test/setup.ts`
  - Remove redundant comments and make fetch-mock policy explicit.
- Test: existing package tests under `packages/core/test`, `packages/api-server/test`, `packages/mcp-server/test`.

## Task 1: Fix MCP JSON-RPC `id: 0` Handling

**Files:**
- Modify: `packages/mcp-server/src/protocol/handler.ts:61-74`
- Test: `packages/mcp-server/test/protocol.test.ts`

- [ ] **Step 1: Write failing test for `id: 0`**

Add to `packages/mcp-server/test/protocol.test.ts`:

```ts
describe("MCP JSON-RPC ids", () => {
  beforeEach(() => {
    global.fetch = mock(() =>
      Promise.resolve(new Response(JSON.stringify([]), { status: 200 })),
    ) as unknown as typeof fetch;
  });

  afterEach(() => {
    global.fetch = originalFetch;
  });

  test("tools/list treats id 0 as a request id", async () => {
    const response = await handleMcpRequest(
      new Request("http://localhost", {
        method: "POST",
        body: JSON.stringify({
          jsonrpc: "2.0",
          id: 0,
          method: "tools/list",
        }),
      }),
    );

    expect(response.status).toBe(200);
    const body = await response.json();
    expect(body.id).toBe(0);
    expect(body.result.tools).toBeArray();
  });
});
```

- [ ] **Step 2: Run test to verify current behavior**

Run: `bun test packages/mcp-server/test/protocol.test.ts`

Expected: FAIL if `id: 0` is treated as notification or no body is returned.

- [ ] **Step 3: Implement minimal fix**

In `packages/mcp-server/src/protocol/handler.ts`, replace notification check:

```ts
const isNotification = !("id" in body);

if (isNotification) {
  if (method === "notifications/initialized") {
    return new Response(null, { status: 204 });
  }
  if (method === "notifications/progress") {
    return new Response(null, { status: 204 });
  }
  return new Response(null, { status: 204 });
}
```

- [ ] **Step 4: Run package test**

Run: `bun test packages/mcp-server/test/protocol.test.ts`

Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add packages/mcp-server/src/protocol/handler.ts packages/mcp-server/test/protocol.test.ts
git commit -m "fix: preserve zero json-rpc ids"
```

## Task 2: Preserve Zero Numeric MCP Arguments

**Files:**
- Modify: `packages/mcp-server/src/protocol/handler.ts:107-216`
- Test: `packages/mcp-server/test/protocol.test.ts`

- [ ] **Step 1: Write failing tests for zero-valued params**

Add to `packages/mcp-server/test/protocol.test.ts`:

```ts
describe("MCP numeric argument forwarding", () => {
  beforeEach(() => {
    global.fetch = mock(() =>
      Promise.resolve(new Response(JSON.stringify([]), { status: 200 })),
    ) as unknown as typeof fetch;
  });

  afterEach(() => {
    global.fetch = originalFetch;
  });

  test("search_kijiji forwards maxPages 0 and price bounds 0", async () => {
    await handleMcpRequest(
      new Request("http://localhost", {
        method: "POST",
        body: JSON.stringify({
          jsonrpc: "2.0",
          id: 1,
          method: "tools/call",
          params: {
            name: "search_kijiji",
            arguments: {
              query: "laptop",
              maxPages: 0,
              priceMin: 0,
              priceMax: 0,
            },
          },
        }),
      }),
    );

    const calledUrl = (global.fetch as unknown as ReturnType<typeof mock>).mock
      .calls[0]?.[0];
    expect(String(calledUrl)).toContain("maxPages=0");
    expect(String(calledUrl)).toContain("priceMin=0");
    expect(String(calledUrl)).toContain("priceMax=0");
  });

  test("search_ebay forwards zero price bounds and maxItems", async () => {
    await handleMcpRequest(
      new Request("http://localhost", {
        method: "POST",
        body: JSON.stringify({
          jsonrpc: "2.0",
          id: 1,
          method: "tools/call",
          params: {
            name: "search_ebay",
            arguments: {
              query: "laptop",
              minPrice: 0,
              maxPrice: 0,
              maxItems: 0,
            },
          },
        }),
      }),
    );

    const calledUrl = (global.fetch as unknown as ReturnType<typeof mock>).mock
      .calls[0]?.[0];
    expect(String(calledUrl)).toContain("minPrice=0");
    expect(String(calledUrl)).toContain("maxPrice=0");
    expect(String(calledUrl)).toContain("maxItems=0");
  });
});
```

- [ ] **Step 2: Run tests to verify failure**

Run: `bun test packages/mcp-server/test/protocol.test.ts`

Expected: FAIL because `0` params are absent.

- [ ] **Step 3: Add helper and update checks**

In `packages/mcp-server/src/protocol/handler.ts`, add near constants:

```ts
function appendStringParam(
  params: URLSearchParams,
  key: string,
  value: unknown,
): void {
  if (typeof value === "string" && value.length > 0) {
    params.append(key, value);
  }
}

function appendNumberParam(
  params: URLSearchParams,
  key: string,
  value: unknown,
): void {
  if (typeof value === "number" && Number.isFinite(value)) {
    params.append(key, value.toString());
  }
}

function appendBooleanParam(
  params: URLSearchParams,
  key: string,
  value: unknown,
): void {
  if (typeof value === "boolean") {
    params.append(key, value.toString());
  }
}
```

Replace param appends with helpers:

```ts
appendStringParam(params, "location", args.location);
appendStringParam(params, "category", args.category);
appendStringParam(params, "keywords", args.keywords);
appendStringParam(params, "sortBy", args.sortBy);
appendStringParam(params, "sortOrder", args.sortOrder);
appendNumberParam(params, "maxPages", args.maxPages);
appendNumberParam(params, "priceMin", args.priceMin);
appendNumberParam(params, "priceMax", args.priceMax);
appendBooleanParam(params, "unstableFilter", args.unstableFilter);
```

Use same pattern for Facebook and eBay numeric/boolean params.

- [ ] **Step 4: Run tests**

Run: `bun test packages/mcp-server/test/protocol.test.ts`

Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add packages/mcp-server/src/protocol/handler.ts packages/mcp-server/test/protocol.test.ts
git commit -m "fix: forward zero-valued mcp params"
```

## Task 3: Remove Cookie Query Path From MCP and API

**Files:**
- Modify: `packages/mcp-server/src/protocol/tools.ts:55-59`
- Modify: `packages/mcp-server/src/protocol/handler.ts:119`
- Modify: `packages/api-server/src/routes/kijiji.ts:65`
- Test: `packages/mcp-server/test/protocol.test.ts`
- Test: `packages/api-server/test/routes.test.ts`

- [ ] **Step 1: Update MCP tests for no cookie exposure**

Replace existing cookie-input test in `packages/mcp-server/test/protocol.test.ts` with:

```ts
test("search tools should not expose cookie inputs", () => {
  const toolNames = ["search_kijiji", "search_facebook", "search_ebay"];

  for (const toolName of toolNames) {
    const tool = tools.find((candidate) => candidate.name === toolName);
    expect(tool?.inputSchema.properties).not.toHaveProperty("cookies");
    expect(tool?.inputSchema.properties).not.toHaveProperty("cookiesSource");
  }
});
```

Add forwarding test:

```ts
test("search_kijiji should not forward cookies query parameters", async () => {
  await handleMcpRequest(
    new Request("http://localhost", {
      method: "POST",
      body: JSON.stringify({
        jsonrpc: "2.0",
        id: 1,
        method: "tools/call",
        params: {
          name: "search_kijiji",
          arguments: {
            query: "laptop",
            cookies: "s=1",
          },
        },
      }),
    }),
  );

  const calledUrl = (global.fetch as unknown as ReturnType<typeof mock>).mock
    .calls[0]?.[0];
  expect(String(calledUrl)).toContain("/kijiji?q=laptop");
  expect(String(calledUrl)).not.toContain("cookies=");
});
```

- [ ] **Step 2: Update API test expectation**

In `packages/api-server/test/routes.test.ts`, replace `kijijiRoute passes cookies query parameter` test with:

```ts
test("kijijiRoute ignores cookies query parameter", async () => {
  const { kijijiRoute } = await import("../src/routes/kijiji");

  await kijijiRoute(
    new Request(
      "http://localhost/api/kijiji?q=laptop&cookies=s%3D1&maxPages=3",
    ),
  );

  expect(fetchKijijiItems).toHaveBeenCalledWith(
    "laptop",
    4,
    "https://www.kijiji.ca",
    {
      location: undefined,
      category: undefined,
      keywords: undefined,
      sortBy: undefined,
      sortOrder: undefined,
      maxPages: 3,
      priceMin: undefined,
      priceMax: undefined,
    },
    {},
  );
});
```

- [ ] **Step 3: Run tests to verify failure**

Run: `bun test packages/mcp-server/test/protocol.test.ts packages/api-server/test/routes.test.ts`

Expected: FAIL because Kijiji cookie query is still exposed/forwarded.

- [ ] **Step 4: Remove Kijiji cookie schema and forwarding**

Delete `cookies` property from `search_kijiji` in `packages/mcp-server/src/protocol/tools.ts`.

Delete this line from `packages/mcp-server/src/protocol/handler.ts`:

```ts
if (args.cookies) params.append("cookies", args.cookies);
```

Delete this field from `searchOptions` in `packages/api-server/src/routes/kijiji.ts`:

```ts
cookies: reqUrl.searchParams.get("cookies") || undefined,
```

- [ ] **Step 5: Run tests**

Run: `bun test packages/mcp-server/test/protocol.test.ts packages/api-server/test/routes.test.ts`

Expected: PASS.

- [ ] **Step 6: Commit**

```bash
git add packages/mcp-server/src/protocol/tools.ts packages/mcp-server/src/protocol/handler.ts packages/mcp-server/test/protocol.test.ts packages/api-server/src/routes/kijiji.ts packages/api-server/test/routes.test.ts
git commit -m "fix: remove cookie query forwarding"
```

## Task 4: Add Strict API Integer Parsing

**Files:**
- Create: `packages/api-server/src/routes/helpers.ts`
- Modify: `packages/api-server/src/routes/facebook.ts`
- Modify: `packages/api-server/src/routes/ebay.ts`
- Modify: `packages/api-server/src/routes/kijiji.ts`
- Test: `packages/api-server/test/routes.test.ts`

- [ ] **Step 1: Write failing API validation tests**

Add to `packages/api-server/test/routes.test.ts`:

```ts
test("facebookRoute rejects non-integer maxItems", async () => {
  const { facebookRoute } = await import("../src/routes/facebook");

  const response = await facebookRoute(
    new Request("http://localhost/api/facebook?q=laptop&maxItems=10abc"),
  );

  expect(response.status).toBe(400);
  const body = await response.json();
  expect(body.message).toBe("Invalid maxItems parameter");
});

test("kijijiRoute rejects decimal maxPages", async () => {
  const { kijijiRoute } = await import("../src/routes/kijiji");

  const response = await kijijiRoute(
    new Request("http://localhost/api/kijiji?q=laptop&maxPages=1.5"),
  );

  expect(response.status).toBe(400);
  const body = await response.json();
  expect(body.message).toBe("Invalid maxPages parameter");
});

test("ebayRoute rejects non-integer price params", async () => {
  const { ebayRoute } = await import("../src/routes/ebay");

  const response = await ebayRoute(
    new Request("http://localhost/api/ebay?q=laptop&minPrice=10abc"),
  );

  expect(response.status).toBe(400);
  const body = await response.json();
  expect(body.message).toBe("Invalid minPrice parameter");
});
```

- [ ] **Step 2: Run tests to verify failure**

Run: `bun test packages/api-server/test/routes.test.ts`

Expected: FAIL because `parseInt` accepts junk/decimals.

- [ ] **Step 3: Create route helper**

Create `packages/api-server/src/routes/helpers.ts`:

```ts
export function getRequiredSearchQuery(req: Request): string | Response {
  const reqUrl = new URL(req.url);
  const query = req.headers.get("query") || reqUrl.searchParams.get("q");

  if (!query) {
    return Response.json(
      { message: "Request didn't have 'query' header or 'q' search parameter!" },
      { status: 400 },
    );
  }

  return query;
}

export function parseNonNegativeIntegerParam(
  searchParams: URLSearchParams,
  name: string,
  defaultValue?: number,
): number | undefined | Response {
  const rawValue = searchParams.get(name);
  if (rawValue === null) {
    return defaultValue;
  }

  const value = Number(rawValue);
  if (!Number.isInteger(value) || value < 0) {
    return Response.json(
      { message: `Invalid ${name} parameter` },
      { status: 400 },
    );
  }

  return value;
}

export function emptySearchResponse(): Response {
  return Response.json(
    { message: "Search didn't return any results!" },
    { status: 404 },
  );
}
```

- [ ] **Step 4: Update routes to use helper**

In each route:

```ts
const reqUrl = new URL(req.url);
const searchQuery = getRequiredSearchQuery(req);
if (searchQuery instanceof Response) return searchQuery;
```

Use integer helper for each numeric param:

```ts
const maxItems = parseNonNegativeIntegerParam(reqUrl.searchParams, "maxItems", 25);
if (maxItems instanceof Response) return maxItems;
```

For optional params:

```ts
const minPrice = parseNonNegativeIntegerParam(reqUrl.searchParams, "minPrice");
if (minPrice instanceof Response) return minPrice;
```

Replace empty-result response blocks with `return emptySearchResponse();`.

- [ ] **Step 5: Run tests**

Run: `bun test packages/api-server/test/routes.test.ts`

Expected: PASS.

- [ ] **Step 6: Run adapter build**

Run: `bun run --cwd packages/api-server build`

Expected: PASS.

- [ ] **Step 7: Commit**

```bash
git add packages/api-server/src/routes/helpers.ts packages/api-server/src/routes/facebook.ts packages/api-server/src/routes/ebay.ts packages/api-server/src/routes/kijiji.ts packages/api-server/test/routes.test.ts
git commit -m "fix: strictly parse route integers"
```

## Task 5: De-Duplicate MCP API Calls

**Files:**
- Modify: `packages/mcp-server/src/protocol/handler.ts`
- Test: `packages/mcp-server/test/protocol.test.ts`

- [ ] **Step 1: Add regression test for successful tool result after helper extraction**

Add to `packages/mcp-server/test/protocol.test.ts`:

```ts
test("tools/call returns API JSON as text content", async () => {
  global.fetch = mock(() =>
    Promise.resolve(
      new Response(JSON.stringify([{ title: "item" }]), { status: 200 }),
    ),
  ) as unknown as typeof fetch;

  const response = await handleMcpRequest(
    new Request("http://localhost", {
      method: "POST",
      body: JSON.stringify({
        jsonrpc: "2.0",
        id: 1,
        method: "tools/call",
        params: {
          name: "search_facebook",
          arguments: { query: "laptop" },
        },
      }),
    }),
  );

  const body = await response.json();
  expect(body.result.content[0].type).toBe("text");
  expect(JSON.parse(body.result.content[0].text)).toEqual([{ title: "item" }]);
});
```

- [ ] **Step 2: Run MCP tests**

Run: `bun test packages/mcp-server/test/protocol.test.ts`

Expected: PASS before refactor.

- [ ] **Step 3: Extract API call helper**

In `packages/mcp-server/src/protocol/handler.ts`, add:

```ts
async function callMarketplaceApi(
  marketplace: string,
  params: URLSearchParams,
): Promise<unknown> {
  const path = `${API_BASE_URL}/${marketplace}`;
  const url = `${path}?${params.toString()}`;

  logger.log(`[MCP] Calling ${marketplace} API`);
  const response = await Promise.race([
    fetch(url),
    new Promise<Response>((_, reject) =>
      setTimeout(
        () => reject(new Error(`Request timed out after ${API_TIMEOUT}ms`)),
        API_TIMEOUT,
      ),
    ),
  ]);

  if (!response.ok) {
    const errorText = await response.text();
    logger.error(`[MCP] ${marketplace} API error ${response.status}: ${errorText}`);
    throw new Error(`API returned ${response.status}: ${errorText}`);
  }

  return response.json();
}
```

Replace three repeated `Promise.race` blocks with:

```ts
result = await callMarketplaceApi("kijiji", params);
```

Use `"facebook"` and `"ebay"` in their branches.

- [ ] **Step 4: Run MCP tests and build**

Run: `bun test packages/mcp-server/test/protocol.test.ts && bun run --cwd packages/mcp-server build`

Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add packages/mcp-server/src/protocol/handler.ts packages/mcp-server/test/protocol.test.ts
git commit -m "refactor: share mcp api calls"
```

## Task 6: Consolidate Core HTTP Fetching

**Files:**
- Modify: `packages/core/src/utils/http.ts`
- Modify: `packages/core/src/scrapers/facebook.ts`
- Modify: `packages/core/src/scrapers/ebay.ts`
- Test: `packages/core/test/http.test.ts`
- Test: `packages/core/test/facebook-core.test.ts`
- Test: `packages/core/test/ebay-core.test.ts`

- [ ] **Step 1: Add shared HTTP test for response URL and deterministic jitter**

Add to `packages/core/test/http.test.ts`:

```ts
test("fetchHtml can return response URL", async () => {
  global.fetch = mock(() =>
    Promise.resolve(
      new Response("<html></html>", {
        status: 200,
        headers: {},
      }),
    ),
  ) as unknown as typeof fetch;

  Object.defineProperty((global.fetch as unknown as ReturnType<typeof mock>).mock.calls[0] ?? {}, "url", {
    value: "https://example.test/final",
  });

  const result = await fetchHtml("https://example.test", 0, {
    includeResponseUrl: true,
    jitter: () => 0,
  });

  expect(result.html).toBe("<html></html>");
  expect(result.responseUrl).toBe("https://example.test");
});
```

If current `Response.url` cannot be set in Bun tests, use a mocked object cast to `Response` instead:

```ts
global.fetch = mock(() =>
  Promise.resolve({
    ok: true,
    status: 200,
    url: "https://example.test/final",
    headers: { get: () => null },
    text: () => Promise.resolve("<html></html>"),
  }),
) as unknown as typeof fetch;
```

- [ ] **Step 2: Run HTTP tests to verify failure**

Run: `bun test packages/core/test/http.test.ts`

Expected: FAIL because options do not exist yet.

- [ ] **Step 3: Extend `FetchHtmlOptions`**

In `packages/core/src/utils/http.ts`:

```ts
export interface FetchHtmlResult {
  html: HTMLString;
  responseUrl: string;
}

export interface FetchHtmlOptions {
  maxRetries?: number;
  retryBaseMs?: number;
  timeoutMs?: number;
  onRateInfo?: (remaining: string | null, reset: string | null) => void;
  headers?: Record<string, string>;
  includeResponseUrl?: boolean;
  jitter?: () => number;
}
```

Update `calculateBackoffDelay`:

```ts
function calculateBackoffDelay(
  attempt: number,
  baseMs: number,
  jitter: () => number,
): number {
  const exponentialDelay = baseMs * 2 ** attempt;
  const jitterDelay = jitter() * 0.1 * exponentialDelay;
  return Math.min(exponentialDelay + jitterDelay, 30000);
}
```

Keep compatible overloads:

```ts
export async function fetchHtml(
  url: string,
  delayMs: number,
  opts: FetchHtmlOptions & { includeResponseUrl: true },
): Promise<FetchHtmlResult>;
export async function fetchHtml(
  url: string,
  delayMs: number,
  opts?: FetchHtmlOptions,
): Promise<HTMLString>;
```

Return either string or result:

```ts
const html = await res.text();
await delay(delayMs);
return opts?.includeResponseUrl ? { html, responseUrl: res.url || url } : html;
```

- [ ] **Step 4: Replace Facebook local HTTP client**

In `packages/core/src/scrapers/facebook.ts`:

Remove local `HttpError` class and local `fetchHtml` function.

Import shared helpers:

```ts
import { fetchHtml, HttpError, isRecord } from "../utils/http";
```

Create Facebook headers constant or local function:

```ts
function createFacebookHeaders(cookies: string): Record<string, string> {
  return {
    accept:
      "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7",
    "accept-language": "en-GB,en-US;q=0.9,en;q=0.8",
    "cache-control": "no-cache",
    "upgrade-insecure-requests": "1",
    "sec-fetch-dest": "document",
    "sec-fetch-mode": "navigate",
    "sec-fetch-site": "none",
    "sec-fetch-user": "?1",
    "user-agent":
      "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36",
    cookie: cookies,
  };
}
```

Replace search fetch:

```ts
const response = await fetchHtml(searchUrl, delayMs, {
  maxRetries: 3,
  includeResponseUrl: true,
  headers: createFacebookHeaders(cookiesHeader),
  onRateInfo: (remaining, reset) => {
    if (remaining && reset) {
      logger.log(
        `\nFacebook - Rate limit remaining: ${remaining}, reset in: ${reset}s`,
      );
    }
  },
});
```

Update error property reads from `err.status` to `err.statusCode`.

- [ ] **Step 5: Replace eBay direct fetch with shared helper**

In `packages/core/src/scrapers/ebay.ts`, import `fetchHtml` and `HttpError` from `../utils/http`.

Replace direct `fetch` block with:

```ts
const searchHtml = await fetchHtml(searchUrl, delayMs, {
  headers,
});
```

Replace `console.error` with:

```ts
logger.error(`Failed to fetch eBay search (${err.statusCode}): ${err.message}`);
```

- [ ] **Step 6: Run core tests**

Run: `bun test packages/core/test/http.test.ts packages/core/test/facebook-core.test.ts packages/core/test/ebay-core.test.ts`

Expected: PASS.

- [ ] **Step 7: Run core build/typecheck**

Run: `bun run --cwd packages/core build && bun run ci`

Expected: PASS.

- [ ] **Step 8: Commit**

```bash
git add packages/core/src/utils/http.ts packages/core/src/scrapers/facebook.ts packages/core/src/scrapers/ebay.ts packages/core/test/http.test.ts packages/core/test/facebook-core.test.ts packages/core/test/ebay-core.test.ts
git commit -m "refactor: share scraper http fetching"
```

## Task 7: Clean Kijiji Dead Code and Logging

**Files:**
- Modify: `packages/core/src/scrapers/kijiji.ts`
- Test: `packages/core/test/kijiji-core.test.ts`
- Test: `packages/core/test/kijiji-integration.test.ts`

- [ ] **Step 1: Verify `_parseListing` has no callers**

Run: `rg "_parseListing|parseListing" packages/core packages/api-server packages/mcp-server`

Expected: only `_parseListing` definition appears. If any caller appears, stop and update this task to preserve behavior.

- [ ] **Step 2: Delete dead function**

Remove `function _parseListing(...)` block from `packages/core/src/scrapers/kijiji.ts`.

- [ ] **Step 3: Replace direct timers and console logging**

Replace:

```ts
await new Promise((resolve) =>
  setTimeout(resolve, DELAY_MS * batchIndex),
);
```

with:

```ts
await delay(DELAY_MS * batchIndex);
```

Replace:

```ts
await new Promise((resolve) => setTimeout(resolve, DELAY_MS));
```

with:

```ts
await delay(DELAY_MS);
```

Replace `console.error(...)` calls with `logger.error(...)` preserving message text.

- [ ] **Step 4: Run Kijiji tests**

Run: `bun test packages/core/test/kijiji-core.test.ts packages/core/test/kijiji-integration.test.ts`

Expected: PASS.

- [ ] **Step 5: Commit**

```bash
git add packages/core/src/scrapers/kijiji.ts
git commit -m "refactor: clean kijiji scraper internals"
```

## Task 8: Clean Test Setup Comments and Enforce Fetch Mocking

**Files:**
- Modify: `packages/core/test/setup.ts`
- Test: core test suite

- [ ] **Step 1: Update setup file**

Replace `packages/core/test/setup.ts` with:

```ts
global.fetch = (() => {
  throw new Error("Tests must mock fetch explicitly");
}) as typeof fetch;
```

- [ ] **Step 2: Run core tests**

Run: `bun test packages/core/test`

Expected: PASS. If failures occur, fix individual tests by mocking `global.fetch` in `beforeEach` and restoring in `afterEach`.

- [ ] **Step 3: Commit**

```bash
git add packages/core/test/setup.ts packages/core/test
git commit -m "test: require explicit fetch mocks"
```

## Task 9: Final Verification

**Files:**
- Verify all touched packages.

- [ ] **Step 1: Run full deterministic tests**

Run: `bun test packages/core/test packages/api-server/test packages/mcp-server/test`

Expected: PASS.

- [ ] **Step 2: Run CI checks**

Run: `bun run ci`

Expected: PASS.

- [ ] **Step 3: Run build**

Run: `bun run build`

Expected: PASS.

- [ ] **Step 4: Inspect diff**

Run: `git diff --stat && git diff --check`

Expected: no whitespace errors, diff limited to planned files.

- [ ] **Step 5: Commit verification fixes if needed**

If verification required fixes:

```bash
git add <fixed-files>
git commit -m "chore: finish code smell cleanup"
```

## Self-Review

- Spec coverage: all reviewed smells are covered: JSON-RPC id bug, zero args, cookie query leak, strict integer parsing, duplicate route/MCP helper code, duplicate HTTP clients, dead Kijiji function, direct timers/logging, stale setup comments.
- Placeholder scan: no TBD/TODO/fill-in placeholders remain. Each task has target files, code snippets, commands, and expected results.
- Type consistency: route helper names, MCP helper names, and shared HTTP option names are used consistently across tasks.

## Execution Handoff

Plan complete and saved to `docs/superpowers/plans/2026-04-28-code-smell-cleanup.md`.

Two execution options:

1. Subagent-Driven (recommended) - dispatch fresh subagent per task, review between tasks, fast iteration.
2. Inline Execution - execute tasks in this session using executing-plans, batch execution with checkpoints.