old4ever/ca-marketplace-scraper
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
main
ca-marketplace-scraper/docs/superpowers/plans/2026-04-28-code-smell-cleanup.md
Dmytro Stanchiev 7ab33d0b02 chore: format markdown 
Signed-off-by: Dmytro Stanchiev <git@dmytros.dev>
2026-05-01 11:42:54 -04:00

27 KiB
Raw Permalink Blame History

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:

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:

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
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:

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:

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:

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
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:

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:

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:

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:

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

Delete this field from searchOptions in packages/api-server/src/routes/kijiji.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
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:

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:

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:

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

Use integer helper for each numeric param:

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

For optional params:

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
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:

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:

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:

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
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:

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:

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:

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:

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:

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:

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:

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

Create Facebook headers constant or local function:

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:

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:

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

Replace console.error with:

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
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:

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

with:

await delay(DELAY_MS * batchIndex);

Replace:

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

with:

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
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:

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
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:

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.