Master plan: address all open issues (17 PRs across 5 phases) #31

New Issue

Claude · 2026-02-15T15:59:00Z

Claude commented

2026-02-15 15:59:00 +00:00

Context

The repository has 30 open issues spanning security vulnerabilities, crash bugs, error handling gaps, code quality improvements, test coverage, and new feature requests. Four epic issues (#1–#4) track groups of sub-tasks. This plan organizes all issues into 17 focused PRs with clear dependency ordering.

Phase 1: Critical & Crash Fixes

PR 1 — Fix XSS vulnerability in SetAttribute (#12)

Branch: fix/escape-javascript-xss

escapeJavaScript() in node.go:107-108 only escapes \ and '. Missing: ", backticks, newlines, null bytes, unicode separators. Replace string interpolation with Playwright's Evaluate argument passing to eliminate the injection surface entirely. Remove or fix escapeJavaScript for defense in depth.

Files: node.go, node_test.go
Closes: #12 → Epic #2

PR 2 — Fix nil-pointer panics (#10, #11)

Branch: fix/nil-pointer-panics

sites/archive/archive.go:107,114 — SelectFirst(...) result immediately dereferenced without nil check. Add nil guards before .Type() and .Click().
document.go:64 — resp.Status() called without checking if resp is nil. Playwright's Reload() can return nil response. Add nil guard.

Files: sites/archive/archive.go, document.go
Closes: #10, #11 → Epic #1

PR 3 — Fix archive cmd panic on short content (#9)

Branch: fix/archive-cmd-short-content

sites/archive/cmd/archive/main.go:116 — article.Content[:32] panics if Content < 32 chars. Use length check (same pattern as cmd/browser/main.go:48-55).

Files: sites/archive/cmd/archive/main.go
Closes: #9 → Epic #1

Phase 2: Error Handling Fixes

PR 4 — Fix DuckDuckGo error handling (#5, #6)

Branch: fix/duckduckgo-error-handling

sites/duckduckgo/duckduckgo.go:133 returns res, nil instead of res, err — discards ForEach error.
sites/duckduckgo/page.go:20 — GetResults() has no error return value; ForEach error silently discarded.

Change SearchPage interface to GetResults() ([]Result, error). Update implementation and all callers. Fix Search() to return res, err.

Files: sites/duckduckgo/page.go, sites/duckduckgo/duckduckgo.go, sites/duckduckgo/cmd/duckduckgo/main.go
Closes: #5, #6 → Epic #1
Must merge before PR 9 (deduplication depends on final interface).

PR 5 — Fix archive cmd flags and defer-before-error-check (#8, #19)

Branch: fix/cmd-flags-and-defer-ordering

sites/archive/cmd/archive/main.go:59 passes Flags (archive only) instead of merged flags variable. Browser flags never registered.
defer DeferClose(x) placed before error check in 6+ locations:
- sites/duckduckgo/cmd/duckduckgo/main.go (browser creation)
- sites/duckduckgo/duckduckgo.go:93 (doc before err check at :95)
- sites/google/cmd/google/main.go (browser creation)
- sites/wegmans/cmd/wegmans/main.go (browser creation)
- sites/wegmans/wegmans.go:64
- sites/aislegopher/aislegopher.go:54

Move each defer after the error check.

Files: sites/archive/cmd/archive/main.go, sites/duckduckgo/cmd/duckduckgo/main.go, sites/duckduckgo/duckduckgo.go, sites/google/cmd/google/main.go, sites/wegmans/cmd/wegmans/main.go, sites/wegmans/wegmans.go, sites/aislegopher/aislegopher.go
Closes: #8, #19 → Epic #1

PR 6 — Fix updateCookies error + context-aware sleep (#7, #18)

Branch: fix/cookies-error-and-context-sleep

playwright.go:140 — error from page.Context().Cookies() never checked before iterating. Add error check.
sites/archive/archive.go:122 — time.Sleep(5s) ignores context. Replace with context-aware select.

Files: playwright.go, sites/archive/archive.go
Closes: #7, #18 → Epic #1

Phase 3: Quality, Consistency & Enhancements

PR 7 — Fix ShowBrowser merge behavior + consistent browser defaults (#16, #15)

Branch: fix/merge-options-and-browser-defaults

browser_init.go:157 — base.ShowBrowser = o.ShowBrowser always overwrites. Change ShowBrowser to *bool in BrowserOptions so nil means "don't override".
NewInteractiveBrowser (interactive.go:63) defaults to Chromium but NewBrowser (playwright.go:95) defaults to Firefox. Align both to Firefox.

Files: playwright.go, browser_init.go, interactive.go, cmd/browser/pkg/browser/flags.go
Closes: #15, #16 → Epic #3

PR 8 — Consolidate user-agent strings (#17)

Branch: fix/user-agent-consistency

Three different UA strings across playwright.go:94 (Firefox/142.0), interactive.go:62 (Firefox/142.0), cmd/browser/pkg/browser/flags.go:20 (Firefox/133.0 — outdated). Define const DefaultUserAgent in a central location, reference it everywhere.

Files: playwright.go, interactive.go, cmd/browser/pkg/browser/flags.go
Closes: #17 → Epic #3

PR 9 — Deduplicate helpers (#13, #14)

Branch: refactor/deduplicate-helpers

Identical numericOnly inline func in sites/powerball/powerball.go and sites/megamillions/megamillions.go. Extract to sites/internal/parse/parse.go.
Result extraction in duckduckgo.go:101-131 nearly identical to page.go:24-54. Extract shared function.

Depends on PR 4 (interface change to GetResults).

Files: new sites/internal/parse/parse.go, sites/powerball/powerball.go, sites/megamillions/megamillions.go, sites/duckduckgo/duckduckgo.go, sites/duckduckgo/page.go
Closes: #13, #14 → Epic #3

PR 10 — Fix silently ignored parsing errors (#24)

Branch: fix/silent-parsing-errors

20+ locations across site extractors discard errors with _ = from Text(), Attr(), strconv.Atoi(), strconv.ParseFloat(). For each location:

Return error for required fields (prices, IDs)
Log warning for optional fields (titles, descriptions)

Depends on PR 9 (deduplication may consolidate some locations).

Key files: sites/wegmans/wegmans.go, sites/aislegopher/aislegopher.go, sites/google/google.go, sites/duckduckgo/duckduckgo.go, sites/duckduckgo/page.go
Closes: #24 → Epic #1

PR 11 — Thread-safe CookieJar, SameSite attr, Google countries (#20, #22, #23)

Branch: enhance/cookies-and-google

cookies_txt.go — staticCookieJar is []Cookie with no sync. Wrap in struct with sync.RWMutex.
cookiejar.go — Cookie struct missing SameSite field. Add it, update conversion functions in playwright.go.
sites/google/google.go:67-81 — only 4 countries. Replace switch with a comprehensive map.

Files: cookies_txt.go, cookies_txt_test.go, cookiejar.go, playwright.go, sites/google/google.go, sites/google/google_test.go
Closes: #20, #22, #23 → Epics #1, #3

Phase 4: Test Infrastructure

PR 12 — Mock-based site extractor tests (#21)

Branch: test/site-extractor-mocks

Create exported mock implementations of Browser, Document, Node in a new extractortest/ package. Write HTML fixture-based tests for 2–3 extractors (duckduckgo, archive, powerball) to establish the pattern. Remaining extractors covered in follow-up.

Files: new extractortest/mock.go, new sites/*/testdata/*.html, new test files
Closes: #21 → Epic #4

Phase 5: New Site Extractors

Branch: feature/duckduckgo-widgets

Extend existing sites/duckduckgo/ with weather.go and stock.go. Add CLI subcommands.
Closes: #25, #26

PR 14 — CoinGecko cryptocurrency extractor (#27)

Branch: feature/coingecko-extractor

New sites/coingecko/ following existing pattern.
Closes: #27

PR 15 — Steam Store game price extractor (#28)

Branch: feature/steam-extractor

New sites/steam/ following existing pattern.
Closes: #28

PR 16 — Allrecipes recipe extractor (#29)

Branch: feature/allrecipes-extractor

New sites/allrecipes/ following existing pattern. Leverage JSON-LD structured data.
Closes: #29

PR 17 — IMDB movie/TV extractor (#30)

Branch: feature/imdb-extractor

New sites/imdb/ following existing pattern.
Closes: #30

Dependency Graph

PR 1 (#12) ─────────── no deps
PR 2 (#10,#11) ──────── no deps
PR 3 (#9) ───────────── no deps
PR 4 (#5,#6) ──────┐── no deps
PR 5 (#8,#19) ─────│── no deps
PR 6 (#7,#18) ─────│── no deps
PR 7 (#16,#15) ────│── no deps
PR 8 (#17) ────────│── after PR 7 (same files)
PR 9 (#13,#14) ────┘── after PR 4
PR 10 (#24) ──────────── after PR 9
PR 11 (#20,#22,#23) ─── no deps
PR 12 (#21) ──────────── after Phases 1-3 stabilize
PRs 13-17 (#25-#30) ─── independent, after PR 12 for test pattern

Epic Closure

Epic #1 (Error Handling): closes after PRs 2, 3, 4, 5, 6, 10
Epic #2 (Security): closes after PR 1 (and PR 11 for SameSite)
Epic #3 (Code Quality): closes after PRs 7, 8, 9, 11
Epic #4 (Test Coverage): closes after PR 12

Verification (per PR)

go build ./... — compilation
go test ./... — all tests pass
Bug fixes: add or update tests that would have caught the bug
New features: include unit tests with mock fixtures
Comment on relevant issues at start/finish milestones

## Context The repository has 30 open issues spanning security vulnerabilities, crash bugs, error handling gaps, code quality improvements, test coverage, and new feature requests. Four epic issues (#1–#4) track groups of sub-tasks. This plan organizes all issues into 17 focused PRs with clear dependency ordering. --- ## Phase 1: Critical & Crash Fixes ### PR 1 — Fix XSS vulnerability in SetAttribute (#12) **Branch:** `fix/escape-javascript-xss` `escapeJavaScript()` in `node.go:107-108` only escapes `\` and `'`. Missing: `"`, backticks, newlines, null bytes, unicode separators. Replace string interpolation with Playwright's `Evaluate` argument passing to eliminate the injection surface entirely. Remove or fix `escapeJavaScript` for defense in depth. **Files:** `node.go`, `node_test.go` **Closes:** #12 → Epic #2 ### PR 2 — Fix nil-pointer panics (#10, #11) **Branch:** `fix/nil-pointer-panics` - `sites/archive/archive.go:107,114` — `SelectFirst(...)` result immediately dereferenced without nil check. Add nil guards before `.Type()` and `.Click()`. - `document.go:64` — `resp.Status()` called without checking if `resp` is nil. Playwright's `Reload()` can return nil response. Add nil guard. **Files:** `sites/archive/archive.go`, `document.go` **Closes:** #10, #11 → Epic #1 ### PR 3 — Fix archive cmd panic on short content (#9) **Branch:** `fix/archive-cmd-short-content` `sites/archive/cmd/archive/main.go:116` — `article.Content[:32]` panics if Content < 32 chars. Use length check (same pattern as `cmd/browser/main.go:48-55`). **Files:** `sites/archive/cmd/archive/main.go` **Closes:** #9 → Epic #1 --- ## Phase 2: Error Handling Fixes ### PR 4 — Fix DuckDuckGo error handling (#5, #6) **Branch:** `fix/duckduckgo-error-handling` - `sites/duckduckgo/duckduckgo.go:133` returns `res, nil` instead of `res, err` — discards `ForEach` error. - `sites/duckduckgo/page.go:20` — `GetResults()` has no error return value; `ForEach` error silently discarded. Change `SearchPage` interface to `GetResults() ([]Result, error)`. Update implementation and all callers. Fix `Search()` to return `res, err`. **Files:** `sites/duckduckgo/page.go`, `sites/duckduckgo/duckduckgo.go`, `sites/duckduckgo/cmd/duckduckgo/main.go` **Closes:** #5, #6 → Epic #1 **Must merge before PR 9** (deduplication depends on final interface). ### PR 5 — Fix archive cmd flags and defer-before-error-check (#8, #19) **Branch:** `fix/cmd-flags-and-defer-ordering` - `sites/archive/cmd/archive/main.go:59` passes `Flags` (archive only) instead of merged `flags` variable. Browser flags never registered. - `defer DeferClose(x)` placed before error check in 6+ locations: - `sites/duckduckgo/cmd/duckduckgo/main.go` (browser creation) - `sites/duckduckgo/duckduckgo.go:93` (doc before err check at :95) - `sites/google/cmd/google/main.go` (browser creation) - `sites/wegmans/cmd/wegmans/main.go` (browser creation) - `sites/wegmans/wegmans.go:64` - `sites/aislegopher/aislegopher.go:54` Move each `defer` after the error check. **Files:** `sites/archive/cmd/archive/main.go`, `sites/duckduckgo/cmd/duckduckgo/main.go`, `sites/duckduckgo/duckduckgo.go`, `sites/google/cmd/google/main.go`, `sites/wegmans/cmd/wegmans/main.go`, `sites/wegmans/wegmans.go`, `sites/aislegopher/aislegopher.go` **Closes:** #8, #19 → Epic #1 ### PR 6 — Fix updateCookies error + context-aware sleep (#7, #18) **Branch:** `fix/cookies-error-and-context-sleep` - `playwright.go:140` — error from `page.Context().Cookies()` never checked before iterating. Add error check. - `sites/archive/archive.go:122` — `time.Sleep(5s)` ignores context. Replace with context-aware select. **Files:** `playwright.go`, `sites/archive/archive.go` **Closes:** #7, #18 → Epic #1 --- ## Phase 3: Quality, Consistency & Enhancements ### PR 7 — Fix ShowBrowser merge behavior + consistent browser defaults (#16, #15) **Branch:** `fix/merge-options-and-browser-defaults` - `browser_init.go:157` — `base.ShowBrowser = o.ShowBrowser` always overwrites. Change `ShowBrowser` to `*bool` in `BrowserOptions` so `nil` means "don't override". - `NewInteractiveBrowser` (`interactive.go:63`) defaults to Chromium but `NewBrowser` (`playwright.go:95`) defaults to Firefox. Align both to Firefox. **Files:** `playwright.go`, `browser_init.go`, `interactive.go`, `cmd/browser/pkg/browser/flags.go` **Closes:** #15, #16 → Epic #3 ### PR 8 — Consolidate user-agent strings (#17) **Branch:** `fix/user-agent-consistency` Three different UA strings across `playwright.go:94` (Firefox/142.0), `interactive.go:62` (Firefox/142.0), `cmd/browser/pkg/browser/flags.go:20` (Firefox/133.0 — outdated). Define `const DefaultUserAgent` in a central location, reference it everywhere. **Files:** `playwright.go`, `interactive.go`, `cmd/browser/pkg/browser/flags.go` **Closes:** #17 → Epic #3 ### PR 9 — Deduplicate helpers (#13, #14) **Branch:** `refactor/deduplicate-helpers` - Identical `numericOnly` inline func in `sites/powerball/powerball.go` and `sites/megamillions/megamillions.go`. Extract to `sites/internal/parse/parse.go`. - Result extraction in `duckduckgo.go:101-131` nearly identical to `page.go:24-54`. Extract shared function. **Depends on PR 4** (interface change to `GetResults`). **Files:** new `sites/internal/parse/parse.go`, `sites/powerball/powerball.go`, `sites/megamillions/megamillions.go`, `sites/duckduckgo/duckduckgo.go`, `sites/duckduckgo/page.go` **Closes:** #13, #14 → Epic #3 ### PR 10 — Fix silently ignored parsing errors (#24) **Branch:** `fix/silent-parsing-errors` 20+ locations across site extractors discard errors with `_ =` from `Text()`, `Attr()`, `strconv.Atoi()`, `strconv.ParseFloat()`. For each location: - **Return error** for required fields (prices, IDs) - **Log warning** for optional fields (titles, descriptions) **Depends on PR 9** (deduplication may consolidate some locations). **Key files:** `sites/wegmans/wegmans.go`, `sites/aislegopher/aislegopher.go`, `sites/google/google.go`, `sites/duckduckgo/duckduckgo.go`, `sites/duckduckgo/page.go` **Closes:** #24 → Epic #1 ### PR 11 — Thread-safe CookieJar, SameSite attr, Google countries (#20, #22, #23) **Branch:** `enhance/cookies-and-google` - `cookies_txt.go` — `staticCookieJar` is `[]Cookie` with no sync. Wrap in struct with `sync.RWMutex`. - `cookiejar.go` — `Cookie` struct missing `SameSite` field. Add it, update conversion functions in `playwright.go`. - `sites/google/google.go:67-81` — only 4 countries. Replace `switch` with a comprehensive map. **Files:** `cookies_txt.go`, `cookies_txt_test.go`, `cookiejar.go`, `playwright.go`, `sites/google/google.go`, `sites/google/google_test.go` **Closes:** #20, #22, #23 → Epics #1, #3 --- ## Phase 4: Test Infrastructure ### PR 12 — Mock-based site extractor tests (#21) **Branch:** `test/site-extractor-mocks` Create exported mock implementations of `Browser`, `Document`, `Node` in a new `extractortest/` package. Write HTML fixture-based tests for 2–3 extractors (duckduckgo, archive, powerball) to establish the pattern. Remaining extractors covered in follow-up. **Files:** new `extractortest/mock.go`, new `sites/*/testdata/*.html`, new test files **Closes:** #21 → Epic #4 --- ## Phase 5: New Site Extractors ### PR 13 — DuckDuckGo weather + stock widget extractors (#25, #26) **Branch:** `feature/duckduckgo-widgets` Extend existing `sites/duckduckgo/` with `weather.go` and `stock.go`. Add CLI subcommands. **Closes:** #25, #26 ### PR 14 — CoinGecko cryptocurrency extractor (#27) **Branch:** `feature/coingecko-extractor` New `sites/coingecko/` following existing pattern. **Closes:** #27 ### PR 15 — Steam Store game price extractor (#28) **Branch:** `feature/steam-extractor` New `sites/steam/` following existing pattern. **Closes:** #28 ### PR 16 — Allrecipes recipe extractor (#29) **Branch:** `feature/allrecipes-extractor` New `sites/allrecipes/` following existing pattern. Leverage JSON-LD structured data. **Closes:** #29 ### PR 17 — IMDB movie/TV extractor (#30) **Branch:** `feature/imdb-extractor` New `sites/imdb/` following existing pattern. **Closes:** #30 --- ## Dependency Graph ``` PR 1 (#12) ─────────── no deps PR 2 (#10,#11) ──────── no deps PR 3 (#9) ───────────── no deps PR 4 (#5,#6) ──────┐── no deps PR 5 (#8,#19) ─────│── no deps PR 6 (#7,#18) ─────│── no deps PR 7 (#16,#15) ────│── no deps PR 8 (#17) ────────│── after PR 7 (same files) PR 9 (#13,#14) ────┘── after PR 4 PR 10 (#24) ──────────── after PR 9 PR 11 (#20,#22,#23) ─── no deps PR 12 (#21) ──────────── after Phases 1-3 stabilize PRs 13-17 (#25-#30) ─── independent, after PR 12 for test pattern ``` ## Epic Closure - **Epic #1** (Error Handling): closes after PRs 2, 3, 4, 5, 6, 10 - **Epic #2** (Security): closes after PR 1 (and PR 11 for SameSite) - **Epic #3** (Code Quality): closes after PRs 7, 8, 9, 11 - **Epic #4** (Test Coverage): closes after PR 12 ## Verification (per PR) 1. `go build ./...` — compilation 2. `go test ./...` — all tests pass 3. Bug fixes: add or update tests that would have caught the bug 4. New features: include unit tests with mock fixtures 5. Comment on relevant issues at start/finish milestones

Claude added the priority/high type/epic labels 2026-02-15 15:59:10 +00:00

Claude commented

2026-02-15 16:11:12 +00:00

Starting work on this master plan. Will begin with Phase 1 (Critical & Crash Fixes) — PRs 1, 2, and 3 — since they have no dependencies and address the most urgent issues.

Plan:

PR 1 — Fix XSS vulnerability in SetAttribute (#12)
PR 2 — Fix nil-pointer panics (#10, #11)
PR 3 — Fix archive cmd panic on short content (#9)

Will work through these sequentially, creating feature branches and PRs for each.

Starting work on this master plan. Will begin with Phase 1 (Critical & Crash Fixes) — PRs 1, 2, and 3 — since they have no dependencies and address the most urgent issues. **Plan:** 1. PR 1 — Fix XSS vulnerability in SetAttribute (#12) 2. PR 2 — Fix nil-pointer panics (#10, #11) 3. PR 3 — Fix archive cmd panic on short content (#9) Will work through these sequentially, creating feature branches and PRs for each.

PR	Branch	Issues	Status
13	`feature/duckduckgo-widgets`	#25, #26	Merged (#44)
14	`feature/coingecko-extractor`	#27	Pending
15	`feature/steam-extractor`	#28	Pending
16	`feature/allrecipes-extractor`	#29	Pending
17	`feature/imdb-extractor`	#30	Pending
18	`feature/bambulab-extractor`	#45	Pending

Master plan: address all open issues (17 PRs across 5 phases) #31