old4ever/ca-marketplace-scraper
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

chore: format markdown

Browse Source 
Signed-off-by: Dmytro Stanchiev <git@dmytros.dev>
This commit is contained in:
Dmytro Stanchiev
2026-05-01 11:42:54 -04:00
parent d2c3c07e7d
commit 7ab33d0b02
15 changed files with 925 additions and 417 deletions
Download Patch File Download Diff File Expand all files Collapse all files

104
docs/superpowers/plans/2026-04-28-code-smell-cleanup.md
View File

@@ -1,14 +1,22 @@
# 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.
> **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.
**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.
**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.
**Tech Stack:** Bun `1.3.13`, TypeScript strict mode, `bun:test`, Biome, framework-free
`Bun.serve` adapters.
---
* * *
## File Structure
@@ -18,7 +26,8 @@
- 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.
- 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.
@@ -53,12 +62,15 @@
- 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`.
- 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`**
@@ -137,7 +149,9 @@ 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**
@@ -288,10 +302,15 @@ 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**
@@ -341,7 +360,8 @@ test("search_kijiji should not forward cookies query parameters", async () => {
- [ ] **Step 2: Update API test expectation**
In `packages/api-server/test/routes.test.ts`, replace `kijijiRoute passes cookies query parameter` test with:
In `packages/api-server/test/routes.test.ts`, replace
`kijijiRoute passes cookies query parameter` test with:
```ts
test("kijijiRoute ignores cookies query parameter", async () => {
@@ -374,13 +394,15 @@ test("kijijiRoute ignores cookies query parameter", async () => {
- [ ] **Step 3: Run tests to verify failure**
Run: `bun test packages/mcp-server/test/protocol.test.ts packages/api-server/test/routes.test.ts`
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 `cookies` property from `search_kijiji` in
`packages/mcp-server/src/protocol/tools.ts`.
Delete this line from `packages/mcp-server/src/protocol/handler.ts`:
@@ -396,7 +418,8 @@ 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`
Run:
`bun test packages/mcp-server/test/protocol.test.ts packages/api-server/test/routes.test.ts`
Expected: PASS.
@@ -410,10 +433,15 @@ 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**
@@ -560,7 +588,9 @@ 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**
@@ -645,7 +675,8 @@ 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`
Run:
`bun test packages/mcp-server/test/protocol.test.ts && bun run --cwd packages/mcp-server build`
Expected: PASS.
@@ -659,11 +690,17 @@ 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**
@@ -695,7 +732,8 @@ test("fetchHtml can return response URL", async () => {
});
```
If current `Response.url` cannot be set in Bun tests, use a mocked object cast to `Response` instead:
If current `Response.url` cannot be set in Bun tests, use a mocked object cast to
`Response` instead:
```ts
global.fetch = mock(() =>
@@ -827,7 +865,8 @@ 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`.
In `packages/core/src/scrapers/ebay.ts`, import `fetchHtml` and `HttpError` from
`../utils/http`.
Replace direct `fetch` block with:
@@ -845,7 +884,8 @@ 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`
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.
@@ -865,15 +905,20 @@ 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`
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.
Expected: only `_parseListing` definition appears.
If any caller appears, stop and update this task to preserve behavior.
- [ ] **Step 2: Delete dead function**
@@ -911,7 +956,8 @@ Replace `console.error(...)` calls with `logger.error(...)` preserving message t
- [ ] **Step 4: Run Kijiji tests**
Run: `bun test packages/core/test/kijiji-core.test.ts packages/core/test/kijiji-integration.test.ts`
Run:
`bun test packages/core/test/kijiji-core.test.ts packages/core/test/kijiji-integration.test.ts`
Expected: PASS.
@@ -925,7 +971,9 @@ 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**
@@ -942,7 +990,8 @@ global.fetch = (() => {
Run: `bun test packages/core/test`
Expected: PASS. If failures occur, fix individual tests by mocking `global.fetch` in `beforeEach` and restoring in `afterEach`.
Expected: PASS. If failures occur, fix individual tests by mocking `global.fetch` in
`beforeEach` and restoring in `afterEach`.
- [ ] **Step 3: Commit**
@@ -954,6 +1003,7 @@ git commit -m "test: require explicit fetch mocks"
## Task 9: Final Verification
**Files:**
- Verify all touched packages.
- [ ] **Step 1: Run full deterministic tests**
@@ -991,9 +1041,13 @@ 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.
- 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
@@ -1001,5 +1055,7 @@ Plan complete and saved to `docs/superpowers/plans/2026-04-28-code-smell-cleanup
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.
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.