From 32e4bbb23e45710d1df6bffa5496f3e6697937e4 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:43:59 +0000 Subject: [PATCH 1/2] Atomic output writes, CSV formula guard, one paid solve per page, 404 is not a block - Output writers (incl. the sidecar and last-attempt file) write a temp file beside the target, fsync and rename; modes follow the umask, existing files keep theirs. - CSV cells beginning with = + - @ TAB CR LF get a leading apostrophe (CSV only); csv_cells_escaped in the sidecar, under the caller's extra. - page_flow.SolveBudget: one paid captcha solve per page across retries and rotations, asked before every purchase in all three engines. - HTTP status reaches the classifier (Playwright, pyppeteer); new not_found state. Amazon's 404 page was read as the throttle page and reported exit 3. - Credential scan judges exclusions on the repo-relative path and fails on 0 files scanned; braced {...} .env values read as unset; negative --proxy-block-retries and negative scraper_api_client counters rejected. - README lists the engine-only flags (checked both ways); references to documents outside the repo removed; stale comment and unused constant gone. Co-Authored-By: Claude Opus 5.5 --- .github/ci_checks.py | 12 +- .gitignore | 4 +- CHANGELOG.md | 74 ++++-- README.md | 26 +- captcha_solver.py | 7 +- env_config.py | 18 ++ fingerprint_client.py | 2 +- output_writer.py | 145 ++++++++++-- page_flow.py | 41 +++- playwright_scraper.py | 57 ++++- product_parser.py | 25 +- puppeteer_scraper.py | 58 ++++- scraper_api_client.py | 10 + selenium_scraper.py | 47 +++- smoke_test.py | 539 +++++++++++++++++++++++++++++++++++++++++- 15 files changed, 979 insertions(+), 86 deletions(-) diff --git a/.github/ci_checks.py b/.github/ci_checks.py index 75b8432..5f8381e 100644 --- a/.github/ci_checks.py +++ b/.github/ci_checks.py @@ -76,7 +76,7 @@ # into pip's vendored code and flags a 32-hex string in `_elffile.py` as # key-shaped. Correct about the string, wrong about the file — and it is the # FIRST thing a new user sees from `python3 smoke_test.py`. A guard people -# have to argue with is one they learn to suppress (CLAUDE.md §22). +# have to argue with is one they learn to suppress. # # Deliberately NOT a narrowing of what gets scanned: an untracked file is # still read, because a key pasted into a scratch file beside the scripts is @@ -107,8 +107,12 @@ def scanned_files(): for path in sorted(REPO.rglob("*")): if not path.is_file() or path.suffix not in SCANNED_SUFFIXES: continue + # Judged on the path INSIDE the repo. `path.parts` of the absolute + # path includes every ancestor, so a clone that happened to live + # under a directory named `venv` (or `.git`) skipped every file and + # reported "nothing credential-shaped" after scanning 0 of them. if any(part in {".git", "__pycache__", ".venv", "venv"} - for part in path.parts): + for part in path.relative_to(REPO).parts): continue if _is_in_virtualenv(path): continue @@ -200,6 +204,10 @@ def secret_check(): failed.append(f"{rel}:{lineno} contains {match[:6]}… — a " f"32-char hex string, the shape of a 2captcha key") + # A scan that read nothing has proved nothing, and must not print "ok". + if scanned == 0: + failed.append("0 files scanned — the exclusions matched everything, " + "so this check proved nothing") if not failed: print(f"ok {scanned} files scanned, nothing credential-shaped") return failed diff --git a/.gitignore b/.gitignore index 1d3e863..c4ec657 100644 --- a/.gitignore +++ b/.gitignore @@ -73,7 +73,7 @@ secrets* # recoverable only because that repo was private at the time. On 2026-09-17 # a sibling had an untracked `.env.bak` holding a 32-hex key in a PUBLIC # repository, one `git add -A` from publication. Craigslist was fixed then -# and the fix never reached anyone else, which is §16's rule unapplied: when -# you fix one, check the siblings. +# and the fix never reached anyone else: when you fix one, check the +# siblings. .env* !.env.example diff --git a/CHANGELOG.md b/CHANGELOG.md index 9be3a8e..ecd7191 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,54 @@ one, and when it does the release notes say so first. ## [Unreleased] +> **Behaviour changes from the 2026-10-14 audit fixes:** +> +> * **A URL Amazon answers with "page not found" (HTTP 404) now exits 5 with +> `stop_reason: not_found`.** It used to be read as the 503 throttle page +> (both carry the same "automated access" comment), retried, rotated and +> reported as **blocked, exit 3**. +> * **CSV cells that begin with `=`, `+`, `-`, `@`, a tab or a line break get a +> leading apostrophe** (CSV only; numbers and the JSON are unchanged). The +> sidecar's `csv_cells_escaped` says how many. +> * **At most one paid captcha solve per page**, across all retries and proxy +> rotations. With a 3-exit pool one page could buy three. +> * **Newly rejected flag values:** a negative `--proxy-block-retries` in all +> three engines; in `scraper_api_client.py` a negative `--retries` or +> `--retry-delay`, and `--timeout` below 1. Each is now exit 2. +> * Output files are now written atomically and keep the permissions plain +> `open()` gave them: new files follow the umask, an existing file keeps its +> mode. + +### Fixed (2026-10-14 audit) + +- **Output writers truncated in place.** `write_json`, `write_csv`, + `write_run_meta` and `write_last_attempt` opened their target with `"w"`, so + a failure mid-write left a truncated file (a good 703-byte `out.json` became + 5 bytes of invalid JSON). They now write a temporary file in the same + directory, fsync it and rename it over the target; a failed write leaves the + previous file byte-identical and no temp file behind. +- **CSV formula injection.** Third-party text (titles, sellers, bullets, review + bodies) reached the CSV as-is. See the blockquote above. +- **No captcha solve budget.** The solve sat inside the per-exit loop with + nothing counting it. `page_flow.SolveBudget` (one per page) is now asked + before every purchase in all three engines. +- **Amazon's 404 read as a throttle.** The Playwright and pyppeteer engines now + pass the navigation's HTTP status to the page classifier, which has a + `not_found` state; Selenium, which has no status to read, recognises + Amazon's own (English) not-found page. +- **The credential scan skipped everything** when the clone lived under a + directory named `venv` or `.git`, because it tested the absolute path. It now + tests the path inside the repo, and fails if it scanned no files at all. +- **A `{...}` placeholder copied from the docs read as configured** in `.env` + (`ws://{login}-zone-…`). Any value still containing one now reads as unset, + with a warning that names the variable, not the value. +- README: the flags the engines do not share are listed, instead of a claim + that all three take the same flags; the offline suite now checks that list + in both directions. +- Removed references to documents outside this repository from comments and + the changelog, and a stale comment about a `--mode variations` that does not + exist, plus the unused `UNIQUE_BY_SKU_MODES` constant. + > **Two changes here affect an existing user before anything else does.** > > **A run that never got its page now exits 5, not 4.** Exit 4 has always @@ -99,7 +147,7 @@ one, and when it does the release notes say so first. sidecar and the most recent attempt both be true. Rewritten on success too, so it is never a stale relic of an old failure. - **A check that binds every call into a shared module against the callee's - real signature** (§17's check #1), which this repo did not have. It catches + real signature**, which this repo did not have. It catches two things nothing else here can: a call whose arguments do not fit the signature, and a call to a name the shared module does not define at all — both of which reach a live run as a crash on the first fetch while import, @@ -126,9 +174,9 @@ one, and when it does the release notes say so first. engine logs the status and does not classify on it). It now reads `http_code`, falling back to `status` only if that is an integer. After the fix, one live call (`--wait-text results` on the README's search URL) answered HTTP 200, upstream 200, 16 rows, status complete. -- **farfetch-scraper leftovers removed from the issue templates, TROUBLESHOOTING, - SECURITY and `diff_runs.py`.** The bug-report tips sent a reader to - Farfetch's `/shopping/kids/items.aspx` hub "which carries no product +- **The issue templates, TROUBLESHOOTING, SECURITY and `diff_runs.py` + describe Amazon.** They had been written for another site: the bug-report + tips sent a reader to Farfetch's `/shopping/kids/items.aspx` hub "which carries no product JSON-LD", the example wrote `--out girls` and expected "96 products"; the site-change template said the parser "tries JSON-LD first" and asked for a JSON-LD dump; TROUBLESHOOTING's 0-rows table named Farfetch's @@ -153,18 +201,6 @@ one, and when it does the release notes say so first. ### CI -- **A check that binds every call into a shared module against the callee's - real signature** (§17's check #1), which this repo did not have. It catches - two things nothing else here can: a call whose arguments do not fit the - signature, and a call to a name the shared module does not define at all — - both of which reach a live run as a crash on the first fetch while import, - `--help`, `compileall` and the undefined-name walk all stay green. It skips - calls using `*args`/`**kwargs` rather than guessing, treats a locally-bound - name as shadowing a same-named module, and asserts it found calls to bind - at all so it cannot pass by scanning nothing. Verified by control. - -### CI - - **A variation canary**, reading one real multi-variant product page and asserting all four columns are populated, that the count is not 1 on a product picked for having many, and that there is one selected value per @@ -208,9 +244,9 @@ one, and when it does the release notes say so first. - **`fingerprint_client.py` could not read the key from `.env`.** `--key` defaulted to `os.environ.get("TWOCAPTCHA_KEY")` and only that, so a key put - in `.env` — exactly as §3, the README and `.env.example` instruct — worked + in `.env` — exactly as the README and `.env.example` instruct — worked for every engine and failed HERE with "No API key". A documented mechanism - not applied on one path, which is the shape of half the defects §16 lists. + not applied on one path. It now reads through `env_config.env_value`, calling `load_env()` itself because this is a standalone entry point that no engine has necessarily run @@ -222,7 +258,7 @@ one, and when it does the release notes say so first. "still set to the placeholder from .env.example" instead. Found on a sibling repo's first live `--fingerprint` run, then checked - across the family before patching, per §16: five repos had it and one had + across the family before patching: five repos had it and one had already fixed it. Pinned by a check verified to fail on the old code — including that the help string does not interpolate its default, which is one substring away from printing a live credential to anyone who types diff --git a/README.md b/README.md index 4f2af0b..29ad24e 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,18 @@ See [`sample_output.json`](sample_output.json) and [`sample_output.csv`](sample_output.csv) — both cut from a real run, not written by hand. CI checks their columns against the schema. +**In the CSV only, a text cell that begins with `=`, `+`, `-`, `@`, a tab or +a line break gets a leading apostrophe**, so a spreadsheet shows it as text +instead of running it as a formula. Titles, sellers, bullets and review text +are written by third parties. Numbers are never touched and the JSON keeps +the original text; `csv_cells_escaped` in `.meta.json` says how many +cells were changed. + +Output files are replaced atomically (written to a temporary file beside the +target, then renamed), so a run killed mid-write leaves the previous file +intact rather than a truncated one. File permissions follow your umask, and +an existing file keeps its mode. + ### The run-metadata sidecar Every successful run writes `.meta.json` beside its output: @@ -225,7 +237,7 @@ way, which is why it is what gets read. | 2 | Bad usage (including a credentialed `--cdp-endpoint` given to Selenium) | | 3 | Blocked before parsing: Amazon's image captcha, an unresolved throttle, or a sign-in wall | | 4 | Ran fine, found nothing | -| 5 | The page was never fetched: a navigation timeout, a dead or unauthenticated proxy, or a Scraper API error. **Not** an empty result — nothing can be concluded about the catalogue | +| 5 | The page was never fetched: a navigation timeout, a dead or unauthenticated proxy, a Scraper API error, or a URL Amazon says does not exist (HTTP 404, `stop_reason: not_found`). **Not** an empty result — nothing can be concluded about the catalogue | | 6 | Partial: some pages gathered, then the run stopped early | Exit 3 covers the sign-in wall as well as a captcha, because the family's @@ -434,7 +446,17 @@ faster way to get that address scored than to gather data). All three engines share the page-state policy (`page_flow.py`) and the status/exit-code mapping (`output_writer.finish_run`), so they cannot drift apart on what a page means or what a run reports. The offline suite asserts -they take the same flags. +that each engine takes the family's core flags; the flag sets are not +identical: + +* `--fingerprint`, `--fp-country` and `--fp-tags` exist in Playwright and + Selenium, not in pyppeteer; `--locale` is Playwright-only. +* pyppeteer adds `--chromium-path`; Selenium adds `--no-sandbox` and + `--disable-dev-shm-usage`. +* `--concurrency` is accepted and ignored by Selenium and pyppeteer (above). + +Paid captcha solves are capped at **one per page** in every engine, however +many retries or proxy rotations that page takes. Limits worth knowing before you pick one: diff --git a/captcha_solver.py b/captcha_solver.py index c8e3bb8..41440d3 100644 --- a/captcha_solver.py +++ b/captcha_solver.py @@ -119,9 +119,8 @@ def detect_recaptcha_v3(html: str, page_url: str) -> Optional[CaptchaChallenge]: attributes (data-captcha-type="recaptcha" data-version="v3" data-sitekey="..." data-action="..."), with the actual execute() call happening inside a bundled JS file, never appearing as - readable inline script text at all — confirmed live on the sibling - farfetch-scraper repo's target, - sign-up modal via DevTools inspection. A previous version of this + readable inline script text at all — confirmed live on another + site's sign-up modal via DevTools inspection. A previous version of this function, which only checked for format 1, reported "no captcha" on this exact page despite one being genuinely present — caught by manually inspecting the DOM, not by the detector itself, which is @@ -170,7 +169,7 @@ def detect_recaptcha_v3(html: str, page_url: str) -> Optional[CaptchaChallenge]: # --------------------------------------------------------------------------- # Runtime detection (added 2026-08-24 — the static-HTML detector below no -# longer matched the sibling farfetch-scraper repo's target) +# longer matched the site it was first written for) # --------------------------------------------------------------------------- # # What changed there: the sign-up modal used to render diff --git a/env_config.py b/env_config.py index a50f953..806537b 100644 --- a/env_config.py +++ b/env_config.py @@ -28,6 +28,7 @@ import logging import os +import re from pathlib import Path logger = logging.getLogger(__name__) @@ -54,6 +55,15 @@ "", } +# A `{...}` left anywhere in a value is a placeholder too. The two +# credentialled URLs are documented the way the vendor documents them — +# `ws://{login}-zone-...:{password}@cb.2captcha.com:9222` and +# `http://{user}:{password}@...` — and neither literal is in the set above, +# so a value copied from the docs read as CONFIGURED and reached the +# endpoint with `{login}` as its username, coming back as a 401 a long way +# from its cause. Real credentials and URLs never contain braces. +_BRACED_PLACEHOLDER = re.compile(r"\{[^{}\s]*\}") + _loaded_from = None @@ -159,6 +169,14 @@ def env_value(name): "%s is still set to the placeholder from .env.example — treating it " "as unset. Put your real value in .env.", name) return None + if _BRACED_PLACEHOLDER.search(stripped): + # Name the variable, never quote the value: it may be a URL with a + # real password beside the placeholder. + logger.warning( + "%s still contains a {...} placeholder from the documentation — " + "treating it as unset. Replace every {...} with your real value.", + name) + return None return stripped diff --git a/fingerprint_client.py b/fingerprint_client.py index e84ce1e..3210607 100644 --- a/fingerprint_client.py +++ b/fingerprint_client.py @@ -331,7 +331,7 @@ def main() -> int: # loader. It used to read `os.environ` alone, which meant a key put in # `.env` exactly as the README and .env.example instruct worked for every # engine and failed HERE with "No API key" — a documented mechanism not - # applied on one path, which is the shape of half the defects §16 lists. + # applied on one path. # # `.env` has to be LOADED before it can be read: `env_value` looks at # os.environ, and `load_env` is what fills that from the file. Calling it diff --git a/output_writer.py b/output_writer.py index 6fb1dd3..0686b6a 100644 --- a/output_writer.py +++ b/output_writer.py @@ -24,8 +24,11 @@ gets the right header for the mode that produced it. """ +import contextlib import csv import json +import os +import tempfile from dataclasses import dataclass, asdict, field, fields from datetime import datetime, timezone from typing import Optional, List, Set, Sequence, Any, Type @@ -124,8 +127,8 @@ class Product: # Scalars rather than the whole variant table, because the table does not # fit in a row: one measured product (Crocs Classic Clog, 2026-09-21) # publishes 776 variants, and 776 objects in one CSV cell is not a - # column anyone can use. The table lives in `--mode variations`, one row - # per variant, the same way reviews got their own schema. + # column anyone can use. The per-variant table is not exported by any + # mode of this repo; these four columns are what a row can carry. parent_asin: Optional[str] = None # The dimensions this product varies over, as the site labels them for a # human: ["Size", "Color"], ["Option", "Digital Storage Capacity", ...]. @@ -177,10 +180,6 @@ class Review: # Row classes by --mode, so an engine maps its mode to a schema in one place. ROW_CLASS_BY_MODE = {"listing": Product, "product": Product, "reviews": Review} -# Modes whose rows are one-per-ASIN, and therefore safe to dedupe on `sku` -# and to hand to diff_runs.py. A reviews run has many rows per ASIN. -UNIQUE_BY_SKU_MODES = ("listing", "product") - def dedupe_by_key(rows: Sequence[Any], seen: Set[str], key: str = "sku") -> List[Any]: """Drop rows whose key already appeared earlier in this same run. @@ -229,12 +228,90 @@ def _csv_value(v: Any) -> Any: return v +@contextlib.contextmanager +def _atomic(path: str, newline: Optional[str] = None): + """Write to a temporary file beside `path`, then rename over it. + + `open(path, "w")` truncates before a single byte is written, so a crash, + a kill, a full disk or a Ctrl-C halfway through `json.dump` leaves a + SHORTER file where a complete one was. Measured here before this + existed: a good 703-byte `out.json` came back as 5 bytes of invalid JSON + after a write that failed part-way. That breaks the promise `save()` + keeps by refusing to overwrite good output with an empty result: the + previous good run was destroyed by the ATTEMPT to replace it. + + * The temp file goes in the target's OWN directory, because `os.replace` + is atomic only within one filesystem. + * `fsync` before the rename makes the bytes durable, not merely visible. + * `newline` is passed through because the csv module needs + `newline=""`, or every row gains a stray CR on Windows. + * The final mode is what `open(path, "w")` would have produced: + `NamedTemporaryFile` creates at 0600 and `os.replace` keeps that, so + without the chmod every output would become owner-only. A new file + gets `0o666 & ~umask`; an existing file keeps its own mode, because + someone may have set it on purpose. + """ + directory = os.path.dirname(os.path.abspath(path)) or "." + handle = tempfile.NamedTemporaryFile( + mode="w", encoding="utf-8", newline=newline, dir=directory, + prefix=os.path.basename(path) + ".", suffix=".tmp", delete=False) + try: + with handle: + yield handle + handle.flush() + os.fsync(handle.fileno()) + try: + mode = os.stat(path).st_mode & 0o777 + except OSError: + umask = os.umask(0) + os.umask(umask) + mode = 0o666 & ~umask + os.chmod(handle.name, mode) + os.replace(handle.name, path) + except BaseException: + # Including KeyboardInterrupt: an interrupted run must not leave + # `out.json.ab12cd.tmp` behind beside the good file. + try: + os.unlink(handle.name) + except OSError: + pass + raise + + +# A spreadsheet treats a cell beginning with one of these as a FORMULA +# rather than as text, and most strings in these rows were written by a +# third party: product titles, brands, seller names, bullet points, review +# authors and review bodies. `=HYPERLINK(...)`, `+cmd|...` and `@SUM(...)` +# are the classic shapes; tab, CR and LF are here because some readers strip +# a leading one and then evaluate whatever followed it. +CSV_FORMULA_LEADS = ("=", "+", "-", "@", "\t", "\r", "\n") + +# Excel, Google Sheets and LibreOffice read a leading apostrophe as "the rest +# of this cell is text" and do not display it. A consumer parsing the file +# with the `csv` module DOES see it, which is why the count goes into the +# sidecar as `csv_cells_escaped`. +CSV_FORMULA_ESCAPE = "'" + + +def _csv_escape(v: Any): + """Neutralise a formula-shaped cell. Returns (value, was_escaped). + + Only STRINGS are touched: escaping a number would turn a negative value + into text and break every sum over the column. CSV only — the JSON output + keeps the site's text exactly as served. + """ + if isinstance(v, str) and v.startswith(CSV_FORMULA_LEADS): + return CSV_FORMULA_ESCAPE + v, True + return v, False + + def write_json(rows: Sequence[Any], path: str) -> None: - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump([asdict(r) for r in rows], f, ensure_ascii=False, indent=2) -def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None: +def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> int: + """Write the CSV; return how many cells were formula-escaped.""" # An empty result still gets the header row. A zero-byte file makes a # consumer fail on read (no columns to parse) instead of reading a valid # table with zero rows — and "an empty result is still a well-formed @@ -243,11 +320,20 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None: # The header comes from `row_cls`, not from the first row, so an empty # reviews run writes reviews columns rather than product columns. fieldnames = [f.name for f in fields(row_cls)] - with open(path, "w", encoding="utf-8", newline="") as f: + escaped = 0 + with _atomic(path, newline="") as f: writer = csv.DictWriter(f, fieldnames=fieldnames) writer.writeheader() for r in rows: - writer.writerow({k: _csv_value(v) for k, v in asdict(r).items()}) + row = {} + for k, v in asdict(r).items(): + # AFTER the list serialiser: a list is not a string, but the + # joined cell it becomes can start with a formula character. + value, was_escaped = _csv_escape(_csv_value(v)) + escaped += was_escaped + row[k] = value + writer.writerow(row) + return escaped # Exit code used when a run completes but produced nothing. Distinct from 1 @@ -311,7 +397,7 @@ def write_run_meta(out_prefix: str, meta: dict) -> str: both complete, and between runs of different `mode`. """ path = f"{out_prefix}.meta.json" - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump(meta, f, ensure_ascii=False, indent=2) print(f"[+] Wrote run metadata -> {path} (status={meta.get('status')})") return path @@ -339,7 +425,7 @@ def write_last_attempt(out_prefix: str, meta: dict) -> str: success. """ path = f"{out_prefix}.last_attempt.json" - with open(path, "w", encoding="utf-8") as f: + with _atomic(path) as f: json.dump(meta, f, ensure_ascii=False, indent=2) return path @@ -347,7 +433,8 @@ def write_last_attempt(out_prefix: str, meta: dict) -> str: def run_meta(status: str, stop_reason: str, pages_requested: int, pages_completed: int, start_url: str, final_url: str, products: int, pages_failed: Optional[List[int]] = None, - mode: str = "listing", source: str = SOURCE_DEFAULT) -> dict: + mode: str = "listing", source: str = SOURCE_DEFAULT, + extra: Optional[dict] = None) -> dict: """Build the metadata dict for a finished run. `status` is the field a consumer branches on: @@ -368,7 +455,7 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, while 4 and 5 succeed. Recording the numbers keeps the sidecar honest about WHICH part of the catalogue is missing, not just how much. """ - return { + meta = { "source": source, "mode": mode, "status": status, @@ -397,10 +484,15 @@ def run_meta(status: str, stop_reason: str, pages_requested: int, "final_url": final_url, "finished_at": datetime.now(timezone.utc).isoformat(), } + if extra: + # Extra keys never overwrite a contract key such as `status`. + meta.update({k: v for k, v in extra.items() if k not in meta}) + return meta def save(rows: Sequence[Any], out_prefix: str, fmt: str, - allow_empty: bool = False, row_cls: Type = Product) -> int: + allow_empty: bool = False, row_cls: Type = Product, + stats: Optional[dict] = None) -> int: """Write JSON/CSV and return a process exit code. Returns 0 when rows were written, EXIT_NO_PRODUCTS when there were none. @@ -430,8 +522,18 @@ def save(rows: Sequence[Any], out_prefix: str, fmt: str, write_json(rows, f"{out_prefix}.json") print(f"[+] Saved {len(rows)} {noun} -> {out_prefix}.json") if fmt in ("csv", "both"): - write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) + escaped = write_csv(rows, f"{out_prefix}.csv", row_cls=row_cls) print(f"[+] Saved {len(rows)} {noun} -> {out_prefix}.csv") + # Through `stats`, not the return value: the return value is an + # exit code. The caller puts it in the sidecar, so a consumer + # comparing the CSV with the JSON can see that they differ and where. + if stats is not None: + stats["csv_cells_escaped"] = escaped + if escaped: + print(f"[!] {escaped} CSV cell(s) began with a formula character " + f"and were prefixed with {CSV_FORMULA_ESCAPE!r} so a " + f"spreadsheet reads them as text. The JSON output is " + f"unchanged — see csv_cells_escaped in the sidecar.") return 0 if rows else EXIT_NO_PRODUCTS @@ -477,7 +579,8 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, pages_requested: int, pages_completed: int, start_url: str, final_url: str, pages_failed: Optional[List[int]] = None, - mode: str = "listing", source: str = SOURCE_DEFAULT) -> int: + mode: str = "listing", source: str = SOURCE_DEFAULT, + extra: Optional[dict] = None) -> int: """Write output + the run-metadata sidecar; return the exit code. Shared by all three browser engines so the status/exit-code mapping @@ -504,7 +607,9 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, # names has a hole for every name nobody added to it. complete = stop_reason in COMPLETE_STOP_REASONS and not pages_failed row_cls = ROW_CLASS_BY_MODE.get(mode, Product) - rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls) + write_stats: dict = {} + rc = save(rows, out_prefix, fmt, allow_empty=allow_empty, row_cls=row_cls, + stats=write_stats) wrote_output = bool(rows) or allow_empty status = "complete" if (rows and complete) else ( @@ -513,7 +618,9 @@ def finish_run(rows: Sequence[Any], out_prefix: str, fmt: str, status=status, stop_reason=stop_reason, pages_requested=pages_requested, pages_completed=pages_completed, pages_failed=pages_failed, mode=mode, source=source, - start_url=start_url, final_url=final_url, products=len(rows)) + start_url=start_url, final_url=final_url, products=len(rows), + # Housekeeping goes UNDER the caller's `extra`, never over it. + extra={**write_stats, **(extra or {})} or None) if wrote_output: write_run_meta(out_prefix, meta) diff --git a/page_flow.py b/page_flow.py index 74af646..4fae598 100644 --- a/page_flow.py +++ b/page_flow.py @@ -323,9 +323,43 @@ def proxy_failure(exc) -> str: return "" +# How many paid captcha solves one page may buy, across EVERY attempt at it: +# transport retries, throttle reloads and rotations to another exit alike. +# Before this existed the solve sat inside the per-exit loop with nothing +# counting it, so a page that kept coming back behind a captcha bought one +# solve per exit: measured with a stubbed solver, a pool of three exits and +# the default --proxy-block-retries 2, ONE page cost THREE solves. A fresh +# exit is a reason to re-fetch, not a fresh allowance. +SOLVES_PER_PAGE = 1 + + +class SolveBudget: + """The per-page allowance of paid solves. Create one per page; never + reset it on a retry or a rotation. + + `spend()` is called immediately BEFORE a purchase and answers whether the + purchase may go ahead, so the check and the count cannot drift apart: a + caller that forgot to count would also have forgotten to ask. + """ + + def __init__(self, limit: int = SOLVES_PER_PAGE): + self.limit = limit + self.spent = 0 + + def spend(self) -> bool: + if self.spent >= self.limit: + logger.warning("This page has already used its captcha-solve " + "allowance (%d) — not buying another. The run " + "will report what the page holds.", self.limit) + return False + self.spent += 1 + return True + + def numeric_arg_errors(*, pages: int, retries: int, retry_delay: float, delay: float, concurrency: Optional[int] = None, - min_score: Optional[float] = None) -> List[str]: + min_score: Optional[float] = None, + proxy_block_retries: Optional[int] = None) -> List[str]: """Every out-of-range numeric argument, as messages. Pure, so it is testable without building an argparse parser. @@ -370,6 +404,11 @@ def numeric_arg_errors(*, pages: int, retries: int, retry_delay: float, errors.append("--delay cannot be negative") if concurrency is not None and concurrency < 1: errors.append("--concurrency must be at least 1") + # 0 is legal here (it counts RE-tries from another exit), but a negative + # value made the per-exit loop run zero times, so the page was never + # fetched and the run went on to judge a page it never loaded. + if proxy_block_retries is not None and proxy_block_retries < 0: + errors.append("--proxy-block-retries cannot be negative") if min_score is not None and not 0.0 <= min_score <= 1.0: errors.append("--min-score is a reCAPTCHA v3 score and must be " "between 0.0 and 1.0") diff --git a/playwright_scraper.py b/playwright_scraper.py index 16f14e5..ad2e4d0 100644 --- a/playwright_scraper.py +++ b/playwright_scraper.py @@ -119,10 +119,14 @@ class PageOutcome: # Set when Amazon answered with a signin redirect. Neither a block nor a # failure: the run asked for something an anonymous visitor cannot have. needs_signin: bool = False + # Set when Amazon answered "this page does not exist" (HTTP 404/410 or its + # own not-found page). Not a block: no exit, proxy or solve changes it. + not_found: bool = False @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None and not self.needs_signin + return (not self.load_failed and self.blocked_by is None + and not self.needs_signin and not self.not_found) ITEM_LINK_SELECTOR = SELECTORS["item_link"] @@ -485,9 +489,13 @@ def _content_when_settled(page, attempts: int = 4, pause_ms: int = 700): return None -def handle_captcha_if_present(page, args) -> bool: +def handle_captcha_if_present(page, args, budget) -> bool: """Detect and solve a challenge. True if something was solved. + `budget` is the page's page_flow.SolveBudget: every purchase below asks + it first, so retries and rotations cannot buy a second solve for the + same page. + Runs after EVERY navigation, for ANY page — not scoped to one URL. Two families are checked, because Amazon serves one on the paths this scraper reads and the other elsewhere: @@ -531,6 +539,8 @@ def handle_captcha_if_present(page, args) -> bool: "Pass --twocaptcha-key or set TWOCAPTCHA_KEY if the " "run comes back blocked (exit 3).") return False + if not budget.spend(): + return False try: # Fetched through the BROWSER's request context, not this # process: the image is bound to the session that was served the @@ -580,6 +590,8 @@ def fetch_image(url): "solved — continuing with whatever the page already " "holds.") return False + if not budget.spend(): + return False try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -631,7 +643,13 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # Zero without a pool: there is nowhere else to go, and a bare retry from # the same address just burns it further. block_retries = args.proxy_block_retries if (pool and len(pool) > 1) else 0 + # One allowance for the whole page, across every exit and retry. + budget = page_flow.SolveBudget() html, state, load_failed = None, "ok", False + # The HTTP status of the navigation that produced the current + # document, or None when the driver cannot say (Selenium) or the + # document has since been replaced (a captcha solve navigates). + status = None for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) @@ -640,7 +658,9 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome load_failed, exit_failed = False, None for attempt in range(1, args.retries + 1): try: - session.page.goto(url, wait_until="domcontentloaded", timeout=60000) + response = session.page.goto(url, wait_until="domcontentloaded", + timeout=60000) + status = response.status if response else None load_failed = False break except (PWTimeout, PWError) as e: @@ -676,13 +696,14 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome # check would be looking at a challenge page rather than at Amazon. _wait_out_waf_challenge(session.page, args) - if handle_captcha_if_present(session.page, args): + if handle_captcha_if_present(session.page, args, budget): # A solve navigated the page. Give the destination a moment # before judging what came back. session.page.wait_for_timeout(1000) + status = None # the solve replaced the document html = _content_when_settled(session.page) or "" - state = detect_page_state(html, url=session.page.url) + state = detect_page_state(html, status=status, url=session.page.url) # A 503 throttle is not a block and not a load failure: Amazon # answered, it just declined to serve content this second. A fresh @@ -697,16 +718,18 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome "(%d/%d).", pause, attempt, args.retries) time.sleep(pause) try: - session.page.goto(url, wait_until="domcontentloaded", timeout=60000) + response = session.page.goto(url, wait_until="domcontentloaded", + timeout=60000) + status = response.status if response else None except (PWTimeout, PWError): continue _wait_out_waf_challenge(session.page, args) html = _content_when_settled(session.page) or "" - state = detect_page_state(html, url=session.page.url) + state = detect_page_state(html, status=status, url=session.page.url) if state != "throttled": break - if state in ("ok", "signin"): + if state in ("ok", "signin", "not_found"): break # Still throttled or still behind a captcha. A different exit is the @@ -724,6 +747,18 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome.load_failed = True return outcome + if state == "not_found": + # The URL names a page Amazon does not have — a delisted or mistyped + # ASIN, a dead category. Not retried and not rotated: a different + # exit gets the same answer. Before this state existed, Amazon's 404 + # page was read as its 503 throttle page (both carry the same + # "automated access" comment), retried, and reported as blocked — + # exit 3, sending the reader after a proxy for a wrong URL. + logger.error("Amazon says this page does not exist (%s). Check the " + "URL; no proxy or captcha solve changes this.", url) + outcome.not_found = True + return outcome + if state == "signin": # Not a block and not solvable. The run asked for a page Amazon does # not serve anonymously — /product-reviews/ does this on page 1. Kept @@ -1051,6 +1086,7 @@ def scrape(args) -> int: if not first.ok: stop_reason = ("page_load_timeout" if first.load_failed else "needs_signin" if first.needs_signin + else "not_found" if first.not_found else f"blocked_{first.blocked_by}") # A signin wall is not a bot challenge, but it has the same # consequence: something stands between the run and the @@ -1091,6 +1127,7 @@ def scrape(args) -> int: worst = min(failed, key=lambda o: o.page_num) stop_reason = ("page_load_timeout" if worst.load_failed else "needs_signin" if worst.needs_signin + else "not_found" if worst.not_found else f"blocked_{worst.blocked_by}") blocked = any(o.blocked_by or o.needs_signin for o in rest) elif exhausted: @@ -1116,6 +1153,7 @@ def scrape(args) -> int: if not outcome.ok: stop_reason = ("page_load_timeout" if outcome.load_failed else "needs_signin" if outcome.needs_signin + else "not_found" if outcome.not_found else f"blocked_{outcome.blocked_by}") blocked = (outcome.blocked_by is not None or outcome.needs_signin) @@ -1333,7 +1371,8 @@ def parse_args(): for message in page_flow.numeric_arg_errors( pages=args.pages, retries=args.retries, retry_delay=args.retry_delay, delay=args.delay, - concurrency=args.concurrency, min_score=args.min_score): + concurrency=args.concurrency, min_score=args.min_score, + proxy_block_retries=args.proxy_block_retries): p.error(message) if not args.url: p.error("no --url given, and AMAZON_URL is not set in the environment " diff --git a/product_parser.py b/product_parser.py index a01dd44..ce935db 100644 --- a/product_parser.py +++ b/product_parser.py @@ -378,6 +378,22 @@ def _first_price(node, marketplace_currency: Optional[str] = None # got it wrong in both directions at once, so the state is read from the URL. SIGNIN_URL_MARKER = "/ap/signin" +# Amazon's "page not found" answer: HTTP 404 with a 2 KB page ("Sorry! We +# couldn't find that page", a Dogs of Amazon picture). Measured on +# amazon.com on 2026-10-14 for /dp/B0DEADDEAD. It carries the same +# "To discuss automated access to Amazon data" comment as the 503 throttle +# page, so without a state of its own it read as THROTTLED: retried, rotated, +# and finally reported as a block (exit 3) for a URL that simply does not +# exist. The status is the primary signal; this text is the fallback for an +# engine that cannot read a status (Selenium). Only the English wording was +# measured, so on another marketplace's language a statusless engine can +# still miss it. +NOT_FOUND_STATUSES = (404, 410) +# Anchored on the image's alt ATTRIBUTE, not the bare sentence, so a review +# that happens to say "couldn't find that page" cannot match. +NOT_FOUND_MARKERS = ('alt="Sorry! We couldn\'t find that page', + 'alt="Sorry! We couldn't find that page') + # Kept under the family's name and shape so the engines' shared logic reads # the same in every repo. Only the states that genuinely stood between the # run and the content are listed: the WAF interstitial is NOT here, because @@ -390,7 +406,8 @@ def _first_price(node, marketplace_currency: Optional[str] = None def detect_page_state(html: str, status: Optional[int] = None, url: Optional[str] = None) -> str: - """One of "ok", "waf_challenge", "captcha", "throttled", "signin". + """One of "ok", "waf_challenge", "captcha", "not_found", "throttled", + "signin". Order matters. A captcha page also mentions "Sorry", and a signin page can be reached from either, so the most specific and most consequential @@ -402,6 +419,9 @@ def detect_page_state(html: str, status: Optional[int] = None, return "captcha" if any(m in html for m in WAF_CHALLENGE_MARKERS): return "waf_challenge" + # Before the throttle check: the 404 page carries a throttle marker too. + if status in NOT_FOUND_STATUSES or any(m in html for m in NOT_FOUND_MARKERS): + return "not_found" if any(m in html for m in THROTTLE_MARKERS) or status == 503: return "throttled" # From the URL the browser ENDED on — see SIGNIN_URL_MARKER. @@ -1206,8 +1226,7 @@ def _first_parsing_price(soup, selectors, mcur): # # On those same three pages the map held 1, 776 and 9 ASINs and # num_total_variations said 1, 776 and 9 — so the site states the answer we -# can check ourselves against, which beats any threshold (§21's totalPages -# trick on a different field). +# can check ourselves against, which beats any threshold. # # The values in each ASIN's list are positional against `dimensions`, which # is why the two are read together and neither is used alone. diff --git a/puppeteer_scraper.py b/puppeteer_scraper.py index a1de739..c462400 100644 --- a/puppeteer_scraper.py +++ b/puppeteer_scraper.py @@ -147,10 +147,14 @@ class PageOutcome: blocked_by: Optional[str] = None load_failed: bool = False needs_signin: bool = False + # Set when Amazon answered "this page does not exist" (HTTP 404/410 or its + # own not-found page). Not a block: no exit, proxy or solve changes it. + not_found: bool = False @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None and not self.needs_signin + return (not self.load_failed and self.blocked_by is None + and not self.needs_signin and not self.not_found) def _mask_credentials(url: str) -> str: @@ -357,9 +361,13 @@ def _next_page_href(session) -> Optional[str]: NEXT_PAGE_SELECTOR)) -def handle_captcha_if_present(session, args) -> bool: +def handle_captcha_if_present(session, args, budget) -> bool: """Detect and solve a challenge. True if something was solved. + `budget` is the page's page_flow.SolveBudget: every purchase below asks + it first, so retries and rotations cannot buy a second solve for the + same page. + Same two families, same order, same "detected is not blocking" rule as the Playwright engine — see its docstring for why the anchor count is checked here rather than after the readiness wait. @@ -385,6 +393,8 @@ def handle_captcha_if_present(session, args) -> bool: logger.warning("No 2captcha API key, so this captcha cannot be " "solved — continuing with whatever the page holds.") return False + if not budget.spend(): + return False try: def fetch_image(url): # Fetched from inside the page, so it goes over the same @@ -430,6 +440,8 @@ def fetch_image(url): if not args.twocaptcha_key: logger.warning("No 2captcha API key, so this challenge cannot be solved.") return False + if not budget.spend(): + return False try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -457,15 +469,22 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome d = _driver(session) block_retries = args.proxy_block_retries if (pool and len(pool) > 1) else 0 + # One allowance for the whole page, across every exit and retry. + budget = page_flow.SolveBudget() html, state, load_failed = None, "ok", False + # The HTTP status of the navigation that produced the current + # document, or None when the driver cannot say (Selenium) or the + # document has since been replaced (a captcha solve navigates). + status = None for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) load_failed, exit_failed = False, None for attempt in range(1, args.retries + 1): try: - bridge.run(page.goto(url, {"waitUntil": "domcontentloaded", - "timeout": 60000})) + response = bridge.run(page.goto(url, {"waitUntil": "domcontentloaded", + "timeout": 60000})) + status = response.status if response else None load_failed = False break except Exception as e: # noqa: BLE001 — pyppeteer raises many types @@ -504,11 +523,12 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome page_flow.wait_out_waf_challenge(d["content"], d["current_url"], d["sleep"]) - if handle_captcha_if_present(session, args): + if handle_captcha_if_present(session, args, budget): time.sleep(1) + status = None # the solve replaced the document html = _content(session) or "" - state = detect_page_state(html, url=page.url) + state = detect_page_state(html, status=status, url=page.url) if state == "throttled": for attempt in range(1, args.retries + 1): @@ -518,17 +538,18 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome "(%d/%d).", pause, attempt, args.retries) time.sleep(pause) try: - bridge.run(page.goto(url, {"waitUntil": "domcontentloaded", - "timeout": 60000})) + response = bridge.run(page.goto( + url, {"waitUntil": "domcontentloaded", "timeout": 60000})) + status = response.status if response else None except Exception: # noqa: BLE001 continue page_flow.wait_out_waf_challenge(d["content"], d["current_url"], d["sleep"]) html = _content(session) or "" - state = detect_page_state(html, url=page.url) + state = detect_page_state(html, status=status, url=page.url) if state != "throttled": break - if state in ("ok", "signin"): + if state in ("ok", "signin", "not_found"): break if block_attempt < block_retries: logger.warning("Page %d came back as %s from %s — retrying from " @@ -544,6 +565,18 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome.load_failed = True return outcome + if state == "not_found": + # The URL names a page Amazon does not have — a delisted or mistyped + # ASIN, a dead category. Not retried and not rotated: a different + # exit gets the same answer. Before this state existed, Amazon's 404 + # page was read as its 503 throttle page (both carry the same + # "automated access" comment), retried, and reported as blocked — + # exit 3, sending the reader after a proxy for a wrong URL. + logger.error("Amazon says this page does not exist (%s). Check the " + "URL; no proxy or captcha solve changes this.", url) + outcome.not_found = True + return outcome + if state == "signin": logger.error("Amazon redirected to sign-in (%s). This page needs an " "account; no proxy or captcha solve changes that.", page.url) @@ -697,6 +730,7 @@ def scrape(args) -> int: if not first.ok: stop_reason = ("page_load_timeout" if first.load_failed else "needs_signin" if first.needs_signin + else "not_found" if first.not_found else f"blocked_{first.blocked_by}") blocked = first.blocked_by is not None or first.needs_signin elif args.mode == "listing": @@ -736,6 +770,7 @@ def scrape(args) -> int: if not outcome.ok: stop_reason = ("page_load_timeout" if outcome.load_failed else "needs_signin" if outcome.needs_signin + else "not_found" if outcome.not_found else f"blocked_{outcome.blocked_by}") blocked = outcome.blocked_by is not None or outcome.needs_signin break @@ -865,7 +900,8 @@ def parse_args(): for message in page_flow.numeric_arg_errors( pages=args.pages, retries=args.retries, retry_delay=args.retry_delay, delay=args.delay, - concurrency=args.concurrency, min_score=args.min_score): + concurrency=args.concurrency, min_score=args.min_score, + proxy_block_retries=args.proxy_block_retries): p.error(message) if not args.url: p.error("no --url given, and AMAZON_URL is not set in the environment " diff --git a/scraper_api_client.py b/scraper_api_client.py index 24d7029..9b9b665 100644 --- a/scraper_api_client.py +++ b/scraper_api_client.py @@ -388,6 +388,16 @@ def parse_args(): "AMAZON_CDP_ENDPOINT": "cdp_url", "AMAZON_URL": "url", }) + # Here --retries counts EXTRA attempts, so 0 is legal (one attempt). A + # negative value is a typo, and a negative --retry-delay made + # time.sleep() raise after a billed attempt. + if args.retries < 0: + p.error("--retries cannot be negative (it counts extra attempts; " + "0 means one attempt)") + if args.retry_delay < 0: + p.error("--retry-delay cannot be negative") + if args.timeout < 1: + p.error("--timeout must be at least 1 second") if not args.url: p.error("no --url given, and AMAZON_URL is not set in the environment " "or in .env.") diff --git a/selenium_scraper.py b/selenium_scraper.py index d4f0db0..1a0e9a5 100644 --- a/selenium_scraper.py +++ b/selenium_scraper.py @@ -90,10 +90,14 @@ class PageOutcome: blocked_by: Optional[str] = None load_failed: bool = False needs_signin: bool = False + # Set when Amazon answered "this page does not exist" (HTTP 404/410 or its + # own not-found page). Not a block: no exit, proxy or solve changes it. + not_found: bool = False @property def ok(self) -> bool: - return not self.load_failed and self.blocked_by is None and not self.needs_signin + return (not self.load_failed and self.blocked_by is None + and not self.needs_signin and not self.not_found) def _mask_credentials(url: str) -> str: @@ -359,9 +363,13 @@ def _next_page_href(session) -> Optional[str]: return None -def handle_captcha_if_present(session, args) -> bool: +def handle_captcha_if_present(session, args, budget) -> bool: """Detect and solve a challenge. True if something was solved. + `budget` is the page's page_flow.SolveBudget: every purchase below asks + it first, so retries and rotations cannot buy a second solve for the + same page. + Same two families, same order, same "detected is not blocking" rule as the Playwright engine. """ @@ -387,6 +395,8 @@ def handle_captcha_if_present(session, args) -> bool: logger.warning("No 2captcha API key, so this captcha cannot be " "solved — continuing with whatever the page holds.") return False + if not budget.spend(): + return False try: def fetch_image(url): # Fetched from inside the page, so it travels over the same @@ -432,6 +442,8 @@ def fetch_image(url): if not args.twocaptcha_key: logger.warning("No 2captcha API key, so this challenge cannot be solved.") return False + if not budget.spend(): + return False try: token = solve_recaptcha(challenge, args.twocaptcha_key, api_version=args.captcha_api, @@ -460,8 +472,14 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome = PageOutcome(page_num=page_num, url=url) d = _driver(session) html, state, load_failed = None, "ok", False + # The HTTP status of the navigation that produced the current + # document, or None when the driver cannot say (Selenium) or the + # document has since been replaced (a captcha solve navigates). + status = None # always: driver.get() returns nothing to read it from block_retries = args.proxy_block_retries if (pool and len(pool) > 1) else 0 + # One allowance for the whole page, across every exit and retry. + budget = page_flow.SolveBudget() for block_attempt in range(block_retries + 1): logger.info("Fetching page %d/%d: %s", page_num, args.pages, url) @@ -498,11 +516,11 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome page_flow.wait_out_waf_challenge(d["content"], d["current_url"], d["sleep"]) - if handle_captcha_if_present(session, args): + if handle_captcha_if_present(session, args, budget): time.sleep(1) html = d["content"]() or "" - state = detect_page_state(html, url=d["current_url"]()) + state = detect_page_state(html, status=status, url=d["current_url"]()) if state == "throttled": for attempt in range(1, args.retries + 1): @@ -518,11 +536,11 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome page_flow.wait_out_waf_challenge(d["content"], d["current_url"], d["sleep"]) html = d["content"]() or "" - state = detect_page_state(html, url=d["current_url"]()) + state = detect_page_state(html, status=status, url=d["current_url"]()) if state != "throttled": break - if state in ("ok", "signin"): + if state in ("ok", "signin", "not_found"): break if block_attempt < block_retries: logger.warning("Page %d came back as %s from %s — retrying from " @@ -537,6 +555,18 @@ def _fetch_one_page(session, args, pool, page_num: int, url: str) -> PageOutcome outcome.load_failed = True return outcome + if state == "not_found": + # The URL names a page Amazon does not have — a delisted or mistyped + # ASIN, a dead category. Not retried and not rotated: a different + # exit gets the same answer. Before this state existed, Amazon's 404 + # page was read as its 503 throttle page (both carry the same + # "automated access" comment), retried, and reported as blocked — + # exit 3, sending the reader after a proxy for a wrong URL. + logger.error("Amazon says this page does not exist (%s). Check the " + "URL; no proxy or captcha solve changes this.", url) + outcome.not_found = True + return outcome + if state == "signin": logger.error("Amazon redirected to sign-in (%s). This page needs an " "account; no proxy or captcha solve changes that.", @@ -685,6 +715,7 @@ def scrape(args) -> int: if not first.ok: stop_reason = ("page_load_timeout" if first.load_failed else "needs_signin" if first.needs_signin + else "not_found" if first.not_found else f"blocked_{first.blocked_by}") blocked = first.blocked_by is not None or first.needs_signin elif args.mode == "listing": @@ -722,6 +753,7 @@ def scrape(args) -> int: if not outcome.ok: stop_reason = ("page_load_timeout" if outcome.load_failed else "needs_signin" if outcome.needs_signin + else "not_found" if outcome.not_found else f"blocked_{outcome.blocked_by}") blocked = outcome.blocked_by is not None or outcome.needs_signin break @@ -862,7 +894,8 @@ def parse_args(): for message in page_flow.numeric_arg_errors( pages=args.pages, retries=args.retries, retry_delay=args.retry_delay, delay=args.delay, - concurrency=args.concurrency, min_score=args.min_score): + concurrency=args.concurrency, min_score=args.min_score, + proxy_block_retries=args.proxy_block_retries): p.error(message) if not args.url: p.error("no --url given, and AMAZON_URL is not set in the environment " diff --git a/smoke_test.py b/smoke_test.py index ae989c8..b1d46cd 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -37,6 +37,7 @@ import ast import contextlib import importlib +import importlib.util import io import inspect import json @@ -1728,8 +1729,7 @@ def test_engines(skips): "2parser") SHIPPED_TEXT_FILES = ("README.md", "TROUBLESHOOTING.md", "CONTRIBUTING.md", - "SECURITY.md", "CHANGELOG.md", ".env.example", - "CLAUDE.md") + "SECURITY.md", "CHANGELOG.md", ".env.example") # Session material and personal data must not come back with the next capture. @@ -1775,10 +1775,9 @@ def test_fingerprint_client_reads_env(): # THE DEFECT THIS PINS, found the first time --fingerprint was run live # here and confirmed present in five sibling repos: `--key` defaulted to # `os.environ.get("TWOCAPTCHA_KEY")` alone. So a key put in `.env` — - # which is exactly what §3, the README and .env.example instruct — worked + # which is exactly what the README and .env.example instruct — worked # for every engine and failed HERE with "No API key". A documented - # mechanism not applied on one path, which is the shape of half the - # defects §16 lists. + # mechanism not applied on one path. src = inspect.getsource(fpc.main) ok &= check("it loads .env itself, rather than hoping an engine did", "env_config.load_env()" in src) @@ -1845,8 +1844,529 @@ def test_wording(): return ok +# --------------------------------------------------------------------------- +# Output files are replaced atomically, and CSV cells cannot run formulas +# --------------------------------------------------------------------------- +def test_atomic_writes_and_csv_formulas(): + group("output files: atomic replace, file mode, CSV formula guard") + ok = True + import output_writer as ow + tmp = tempfile.mkdtemp() + + # Every writer in output_writer goes through _atomic: none may open a + # file for writing itself. Found by behaviour below as well; this half + # names the writer that regressed. + tree = ast.parse(open(os.path.join(REPO_ROOT, "output_writer.py"), + encoding="utf-8").read()) + writers, raw_opens = [], [] + for fn in [n for n in tree.body if isinstance(n, ast.FunctionDef)]: + src = ast.dump(fn) + if "_atomic" in src: + writers.append(fn.name) + for node in ast.walk(fn): + if (isinstance(node, ast.Call) and getattr(node.func, "id", None) == "open" + and fn.name != "_atomic"): + raw_opens.append(fn.name) + ok &= check("write_json, write_csv, write_run_meta and write_last_attempt " + "all write through _atomic", + {"write_json", "write_csv", "write_run_meta", + "write_last_attempt"} <= set(writers)) + ok &= check("no writer opens its target directly (found: %s)" + % (raw_opens or "none"), not raw_opens) + + # A write that fails part-way leaves the previous good file untouched + # and no temp file behind. Before _atomic a 703-byte good file came back + # as 5 bytes of invalid JSON. + real_dump = json.dump + + def failing_dump(obj, fp, **kw): + fp.write('[{"sk') + raise OSError("disk full (simulated)") + + for writer, target, call in ( + ("write_json", os.path.join(tmp, "good.json"), + lambda p: ow.write_json([Product(sku="B000000002")], p)), + ("write_run_meta", os.path.join(tmp, "good.meta.json"), + lambda p: ow.write_run_meta(p[:-len(".meta.json")], {"status": "x"})), + ("write_last_attempt", os.path.join(tmp, "good.last_attempt.json"), + lambda p: ow.write_last_attempt(p[:-len(".last_attempt.json")], + {"status": "x"}))): + with open(target, "w", encoding="utf-8") as f: + f.write('{"previous": "good"}') + before = open(target, encoding="utf-8").read() + ow.json.dump = failing_dump + try: + with contextlib.redirect_stdout(io.StringIO()): + call(target) + raised = False + except OSError: + raised = True + finally: + ow.json.dump = real_dump + leftovers = [n for n in os.listdir(tmp) if n.endswith(".tmp")] + ok &= check("%s: a failed write raises, keeps the previous file " + "byte-identical and leaves no temp file" % writer, + raised and open(target, encoding="utf-8").read() == before + and not leftovers) + + # File mode: what open(path, "w") would give. NamedTemporaryFile alone + # would make every output 0600. + old_umask = os.umask(0o002) + try: + fresh = os.path.join(tmp, "fresh.json") + ow.write_json([], fresh) + mode_new = os.stat(fresh).st_mode & 0o777 + kept = os.path.join(tmp, "kept.json") + with open(kept, "w") as f: + f.write("[]") + os.chmod(kept, 0o640) + ow.write_json([], kept) + mode_kept = os.stat(kept).st_mode & 0o777 + finally: + os.umask(old_umask) + ok &= check("a new file honours the umask (0664 under 002, got %o)" % mode_new, + mode_new == 0o664) + ok &= check("an existing file keeps its own mode (0640, got %o)" % mode_kept, + mode_kept == 0o640) + + # CSV formula neutralisation: strings only, after list joining, CSV only. + rows = [Product(sku="B000000003", title='=HYPERLINK("http://x","y")', + brand="-5", seller="@SUM(A1)", coupon="\tTab", + bullets=["=cmd|' /C calc'!A0", "fine"], price=-5.0, + category="plain")] + prefix = os.path.join(tmp, "formula") + with contextlib.redirect_stdout(io.StringIO()): + rc = finish_run(rows, prefix, "both", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", + mode="product", extra={"csv_cells_escaped": "caller"}) + import csv as _csv + with open(prefix + ".csv", newline="", encoding="utf-8") as f: + row = list(_csv.DictReader(f))[0] + data = json.load(open(prefix + ".json", encoding="utf-8"))[0] + ok &= check("exit 0 for a written run", rc == 0) + ok &= check("=, -, @ and TAB leading strings get an apostrophe in CSV", + row["title"].startswith("'=") and row["brand"] == "'-5" + and row["seller"] == "'@SUM(A1)" and row["coupon"] == "'\tTab") + ok &= check("a joined list whose first element is a formula is escaped", + row["bullets"].startswith("'=cmd")) + ok &= check("numbers stay numbers (price -5.0 is not escaped)", + row["price"] == "-5.0") + ok &= check("ordinary strings are untouched", row["category"] == "plain") + ok &= check("JSON keeps the site's text exactly", + data["title"].startswith("=HYPERLINK") and data["brand"] == "-5") + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + ok &= check("the caller's extra wins over the writer's own count", + meta.get("csv_cells_escaped") == "caller") + with contextlib.redirect_stdout(io.StringIO()): + finish_run(rows, prefix, "csv", False, blocked=False, + stop_reason="completed", pages_requested=1, + pages_completed=1, start_url="u", final_url="u", mode="product") + meta = json.load(open(prefix + ".meta.json", encoding="utf-8")) + ok &= check("the sidecar declares how many CSV cells were escaped (5)", + meta.get("csv_cells_escaped") == 5 and meta["status"] == "complete") + return ok + + +# --------------------------------------------------------------------------- +# One paid solve per page, however many exits or retries the page takes +# --------------------------------------------------------------------------- +def _solver_calls_are_budgeted(path): + """For each engine file: every call to a paid solver sits in a `try:` + that is immediately preceded by `if not budget.spend(): return False`, + the captcha handler is called with the page's budget, and that budget is + created ONCE per page, outside the per-exit loop. Read off disk, so it + runs whether or not the engine's driver is installed.""" + tree = ast.parse(open(path, encoding="utf-8").read()) + problems = [] + solver_calls = 0 + for fn in [n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef)]: + for parent in ast.walk(fn): + body = getattr(parent, "body", None) + if not isinstance(body, list): + continue + for i, stmt in enumerate(body): + if not isinstance(stmt, ast.Try): + continue + calls = [c for c in ast.walk(stmt) if isinstance(c, ast.Call) + and getattr(c.func, "id", "") in + ("solve_amazon_captcha", "solve_recaptcha")] + if not calls: + continue + solver_calls += len(calls) + prev = body[i - 1] if i else None + guarded = (isinstance(prev, ast.If) + and "budget" in ast.dump(prev.test) + and "spend" in ast.dump(prev.test) + and isinstance(prev.body[0], ast.Return)) + if not guarded: + problems.append("%s line %d: solver call not behind " + "budget.spend()" % (fn.name, stmt.lineno)) + fetch = [n for n in tree.body if isinstance(n, ast.FunctionDef) + and n.name == "_fetch_one_page"] + if not fetch: + problems.append("no _fetch_one_page") + else: + fn = fetch[0] + created = [s for s in fn.body if isinstance(s, ast.Assign) + and "SolveBudget" in ast.dump(s.value)] + if not created: + problems.append("_fetch_one_page does not create its SolveBudget " + "at function level (once per page)") + handler_calls = [c for c in ast.walk(fn) if isinstance(c, ast.Call) + and getattr(c.func, "id", "") == "handle_captcha_if_present"] + if not handler_calls or any( + not any(getattr(a, "id", "") == "budget" for a in c.args) + for c in handler_calls): + problems.append("handle_captcha_if_present not called with the " + "page's budget") + return solver_calls, problems + + +def test_solve_budget(): + group("captcha: one paid solve per page, across retries and rotations") + ok = True + b = page_flow.SolveBudget() + ok &= check("the allowance is 1 per page", + page_flow.SOLVES_PER_PAGE == 1 and b.limit == 1) + with contextlib.redirect_stderr(io.StringIO()): + first, second, third = b.spend(), b.spend(), b.spend() + ok &= check("the first spend is allowed and later ones are refused", + first is True and second is False and third is False + and b.spent == 1) + for name in ENGINES: + calls, problems = _solver_calls_are_budgeted( + os.path.join(REPO_ROOT, name + ".py")) + ok &= check("%s: every paid solve asks the page's budget first " + "(%d solver calls; %s)" % (name, calls, + "; ".join(problems) or "clean"), + calls == 2 and not problems) + return ok + + +def _drive_playwright_fetch(mod, html, status, pool, solver_calls, tmp): + """Run playwright_scraper._fetch_one_page against a fake page. The solver + is replaced by a counter, so nothing is ever sent anywhere.""" + class Resp: + def __init__(self, s): + self.status = s + + class FakePage: + url = "https://www.amazon.com/s?k=x" + + def goto(self, url, **kw): + return Resp(status) + + def content(self): + return html + + def query_selector_all(self, sel): + return [] + + def wait_for_timeout(self, ms): + pass + + def evaluate(self, *a): + return None + + def screenshot(self, **kw): + pass + + def reload(self, **kw): + pass + + class FakeSession: + def __init__(self): + self.page = FakePage() + + def relaunch(self): + self.page = FakePage() + + def fake_solver(*a, **kw): + solver_calls.append(1) + return "abcdef" + + saved = {k: getattr(mod, k) for k in + ("solve_amazon_captcha", "amazon_captcha_submit_url", + "_wait_out_waf_challenge")} + mod.solve_amazon_captcha = fake_solver + mod.amazon_captcha_submit_url = lambda ch, sol: "https://www.amazon.com/s?k=x" + mod._wait_out_waf_challenge = lambda page, args: True + args = types.SimpleNamespace( + pages=1, retries=1, retry_delay=0, proxy_block_retries=2, + solve_captcha="always", twocaptcha_key="stub-never-sent", + captcha_api="v2", min_score=0.3, mode="listing", dump_html=None, + out=os.path.join(tmp, "probe"), category=None) + try: + with contextlib.redirect_stderr(io.StringIO()): + return mod._fetch_one_page(FakeSession(), args, pool, 1, + "https://www.amazon.com/s?k=x") + finally: + for k, v in saved.items(): + setattr(mod, k, v) + + +def test_engine_fetch_behaviour(skips): + group("engine behaviour with the driver faked (Playwright)") + ok = True + try: + mod = importlib.import_module("playwright_scraper") + except ImportError as e: + # Reported by test_engines already; the static halves of these checks + # (test_solve_budget, test_not_found_page) run regardless. + print(" SKIP behavioural fetch checks: %s" % e) + return ok + tmp = tempfile.mkdtemp() + pool = ProxyPool(["http://a:1", "http://b:2", "http://c:3"], rotate="per-run") + calls = [] + out = _drive_playwright_fetch(mod, CAPTCHA_PAGE, 200, pool, calls, tmp) + ok &= check("a page that stays behind a captcha across 3 exits buys ONE " + "solve, not 3 (bought %d)" % len(calls), len(calls) == 1) + ok &= check("...and is reported as blocked", + out.blocked_by == "amazon-captcha") + calls = [] + out = _drive_playwright_fetch(mod, NOT_FOUND_PAGE, 404, pool, calls, tmp) + ok &= check("a 404 is not_found: not blocked, no solve, not retried", + out.not_found and out.blocked_by is None and not calls + and not out.ok) + calls = [] + out = _drive_playwright_fetch(mod, NOT_FOUND_PAGE, None, None, calls, tmp) + ok &= check("with no pool at all the same page is still not_found", + out.not_found and out.blocked_by is None) + return ok + + +# Amazon's own 404 answer for /dp/B0DEADDEAD on amazon.com, 2026-10-14 +# (HTTP 404, 2,319 bytes), trimmed of its inline CSS. It carries the same +# "automated access" comment as the 503 throttle page, which is why it used +# to be read as throttled and reported as blocked. +NOT_FOUND_PAGE = """ + + + + + Page Not Found + + + + + + Amazon + +
+
+ + Sorry! We couldn't find that page. Try searching or go to Amazon's home page. + +
+ + Dogs of Amazon + +
+ + +""" + + +def test_not_found_page(): + group("a page that does not exist is not a block") + ok = True + ok &= check("Amazon's 404 page is not_found, not throttled", + detect_page_state(NOT_FOUND_PAGE) == "not_found") + ok &= check("...and is not reported as a block (it used to be exit 3)", + detect_bot_challenge(NOT_FOUND_PAGE) is None) + ok &= check("an HTTP 404 or 410 is not_found whatever the body says", + detect_page_state("x", status=404) == "not_found" + and detect_page_state("x", status=410) == "not_found") + ok &= check("the 503 throttle page is still throttled", + detect_page_state(FIX_THROTTLE_PAGE_DE) == "throttled" + and detect_page_state(FIX_THROTTLE_PAGE_DE, status=503) == "throttled") + ok &= check("a review merely SAYING the phrase is not a 404 page", + detect_page_state("

I couldn't find that page in the manual" + "

") == "ok") + # Every engine hands the status it has to the classifier, and has a + # not_found outcome with its own stop_reason. Read off disk. + for name in ENGINES: + src = open(os.path.join(REPO_ROOT, name + ".py"), encoding="utf-8").read() + tree = ast.parse(src) + fetch = [n for n in tree.body if isinstance(n, ast.FunctionDef) + and n.name == "_fetch_one_page"][0] + classify = [c for c in ast.walk(fetch) if isinstance(c, ast.Call) + and getattr(c.func, "id", "") == "detect_page_state"] + ok &= check("%s passes a status to every detect_page_state call (%d)" + % (name, len(classify)), + classify and all(any(k.arg == "status" for k in c.keywords) + for c in classify)) + ok &= check("%s records not_found and reports it as its own stop_reason" + % name, "outcome.not_found = True" in src + and src.count('"not_found" if ') >= 2) + ok &= check("a zero-row run that stopped on not_found exits 5, not 3 or 4", + _exit_for([], "not_found") == EXIT_FETCH_FAILED) + return ok + + +def _exit_for(rows, stop_reason): + prefix = os.path.join(tempfile.mkdtemp(), "x") + with contextlib.redirect_stdout(io.StringIO()): + return finish_run(rows, prefix, "json", False, blocked=False, + stop_reason=stop_reason, pages_requested=1, + pages_completed=0, start_url="u", final_url="u") + + +# --------------------------------------------------------------------------- +# Small configuration and CLI guards +# --------------------------------------------------------------------------- +def test_config_and_cli_guards(): + group("placeholders, negative flags and the credential scan's paths") + ok = True + saved = {k: os.environ.get(k) for k in ("AMAZON_PROXY", "AMAZON_CDP_ENDPOINT")} + try: + os.environ["AMAZON_PROXY"] = "http://{user}:{password}@xx.proxy.example:2334" + os.environ["AMAZON_CDP_ENDPOINT"] = ("ws://{login}-zone-scraping_browser-" + "country-xx-pid-{profileId}:{password}" + "@cb.example:9222") + with contextlib.redirect_stderr(io.StringIO()): + proxy = env_config.env_value("AMAZON_PROXY") + cdp = env_config.env_value("AMAZON_CDP_ENDPOINT") + finally: + for k, v in saved.items(): + if v is None: + os.environ.pop(k, None) + else: + os.environ[k] = v + ok &= check("a value still holding a {...} placeholder reads as unset", + proxy is None and cdp is None) + + errors = page_flow.numeric_arg_errors(pages=1, retries=1, retry_delay=0, + delay=0, proxy_block_retries=-1) + ok &= check("--proxy-block-retries -1 is refused and named", + len(errors) == 1 and "--proxy-block-retries" in errors[0]) + ok &= check("--proxy-block-retries 0 is legal (it counts RE-tries)", + page_flow.numeric_arg_errors(pages=1, retries=1, retry_delay=0, + delay=0, proxy_block_retries=0) == []) + for name in ENGINES: + src = open(os.path.join(REPO_ROOT, name + ".py"), encoding="utf-8").read() + ok &= check("%s validates --proxy-block-retries" % name, + "proxy_block_retries=args.proxy_block_retries" in src) + for flag, value in (("--retries", "-1"), ("--retry-delay", "-1"), + ("--timeout", "0")): + proc = subprocess.run([sys.executable, "scraper_api_client.py", "--url", + "https://www.amazon.com/s?k=x", flag, value], + cwd=REPO_ROOT, capture_output=True, text=True, + env=dict(os.environ, TWOCAPTCHA_KEY="")) + err = [l for l in proc.stderr.splitlines() if "error:" in l] + ok &= check("scraper_api_client %s %s is a usage error naming the flag" + % (flag, value), + proc.returncode == 2 and err and flag in err[0]) + + # The credential scan judges exclusions on the path INSIDE the repo. A + # clone living under a directory called `venv` used to scan 0 files and + # report clean. Skipped only when the whole .github directory is absent + # (an image built without CI material), never when one file is. + gh = os.path.join(REPO_ROOT, ".github") + if not os.path.isdir(gh): + print(" SKIP credential-scan path checks: no .github in this tree") + return ok + spec = importlib.util.spec_from_file_location( + "ci_checks_probe", os.path.join(gh, "ci_checks.py")) + cc = importlib.util.module_from_spec(spec) + spec.loader.exec_module(cc) + from pathlib import Path + base = Path(tempfile.mkdtemp()) / "venv" / "clone" + (base / "sub").mkdir(parents=True) + (base / "notes.txt").write_text("nothing here\n") + (base / "sub" / "x.py").write_text("x = 1\n") + env = base / "myenv" + (env / "lib").mkdir(parents=True) + (env / "pyvenv.cfg").write_text("home = /usr\n") + (env / "lib" / "vendored.py").write_text("y = 2\n") + cc.REPO = base + cc._VENV_CACHE.clear() + names = sorted(p.relative_to(base).as_posix() for p in cc.scanned_files()) + ok &= check("a clone under an ancestor named venv still scans its files " + "(%s)" % names, names == ["notes.txt", "sub/x.py"]) + return ok + + +# The flags the engines do NOT share, as the README documents them. Checked +# in both directions: a new unshared flag fails, and so does closing one of +# these differences without updating the README. +ENGINE_ONLY_FLAGS = { + "playwright_scraper": {"--fingerprint", "--fp-country", "--fp-tags", + "--locale"}, + "puppeteer_scraper": {"--chromium-path"}, + "selenium_scraper": {"--fingerprint", "--fp-country", "--fp-tags", + "--no-sandbox", "--disable-dev-shm-usage"}, +} + + +def test_engine_flag_parity(): + group("engine flags: shared set plus the documented differences") + ok = True + flags = {} + for name in ENGINES: + src = open(os.path.join(REPO_ROOT, name + ".py"), encoding="utf-8").read() + flags[name] = set(re.findall(r'add_argument\(\s*"(--[a-z0-9-]+)"', src)) + shared = set.intersection(*flags.values()) + for name in ENGINES: + own = flags[name] - shared + ok &= check("%s: flags beyond the shared set are exactly the documented " + "ones (%s)" % (name, ", ".join(sorted(own)) or "none"), + own == ENGINE_ONLY_FLAGS[name]) + ok &= check("...and the shared set is not empty (%d flags)" % len(shared), + len(shared) > 20) + readme = open(os.path.join(REPO_ROOT, "README.md"), encoding="utf-8").read() + ok &= check("README lists every engine-only flag", + all(f in readme for v in ENGINE_ONLY_FLAGS.values() for f in v)) + return ok + + +# --------------------------------------------------------------------------- +# No private references in shipped files +# --------------------------------------------------------------------------- +# The tokens are assembled from pieces so this file does not match itself. +_SECTION_SIGN = chr(0xA7) +_PRIVATE_TOKENS = ("CLAUDE" + ".md", "claude" + "_2scraper", + "2scraper" + "_repo_rules", "play" + "book " + _SECTION_SIGN, + "template " + _SECTION_SIGN) +_SECTION_REF = re.compile(_SECTION_SIGN + r"\s*\d") + + +def test_no_private_references(): + group("shipped files cite nothing a reader cannot open") + ok = True + scanned = 0 + hits = [] + for root, dirs, files in os.walk(REPO_ROOT): + dirs[:] = [d for d in dirs if d not in (".git", "__pycache__") + and not os.path.isfile(os.path.join(root, d, "pyvenv.cfg"))] + for name in files: + if not (name.endswith((".py", ".md", ".yml", ".yaml", ".txt", + ".toml", ".example", ".cfg")) + or name in (".gitignore", ".dockerignore", "Dockerfile")): + continue + path = os.path.join(root, name) + rel = os.path.relpath(path, REPO_ROOT) + if rel == "CLAUDE" + ".md": + continue # a local-only file, never committed + text = open(path, encoding="utf-8", errors="replace").read() + scanned += 1 + for token in _PRIVATE_TOKENS: + if token in text: + hits.append("%s: %s" % (rel, token)) + if _SECTION_REF.search(text): + hits.append("%s: section sign followed by a number" % rel) + ok &= check("no shipped file cites a private document (%s)" + % ("; ".join(hits) or "none"), not hits) + ok &= check("...and the scan read files (%d)" % scanned, scanned > 20) + return ok + + def test_shared_calls_bind_against_the_real_signature(): - """§17's check #1: bind every call into a shared module against the + """Bind every call into a shared module against the callee's real signature. This exists because of a defect that no other check in this suite can @@ -2218,6 +2738,13 @@ def main() -> int: ok &= test_workflow_inline_python_compiles() ok &= test_fingerprint_client_reads_env() ok &= test_wording() + ok &= test_atomic_writes_and_csv_formulas() + ok &= test_solve_budget() + ok &= test_not_found_page() + ok &= test_engine_fetch_behaviour(skips) + ok &= test_config_and_cli_guards() + ok &= test_engine_flag_parity() + ok &= test_no_private_references() ok &= test_shared_calls_bind_against_the_real_signature() ok &= test_sample_output() From d691d9ee4e37422848600b0140c74541be964036 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:46:39 +0000 Subject: [PATCH 2/2] Canary comments: a refused or skipped live run must fail, not skip green The daily canary already runs ungated from a bare runner and fails on exit 3/4/5/6 or zero rows (last 10 scheduled runs green; 2026-10-14: 77 rows, 3/3 pages, complete). Two comments still recommended a green ::notice:: skip if runners were ever refused; they now say to require AMAZON_PROXY and fail with an ::error:: naming it instead. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/canary.yml | 9 ++++++--- .github/workflows/weekly.yml | 5 +++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/canary.yml b/.github/workflows/canary.yml index b17ca50..ee73869 100644 --- a/.github/workflows/canary.yml +++ b/.github/workflows/canary.yml @@ -236,9 +236,12 @@ jobs: # product, three numbers, two hours. A canary pinned to any of them would # go red for the catalogue behaving normally. # - # If it does start being refused from runners, gate its steps on - # AMAZON_PROXY the way this file's header describes and let it SKIP with a - # ::notice:: when the secret is absent, rather than leaving it red. + # If it does start being refused from runners, set the AMAZON_PROXY secret + # (see this file's header). Do NOT make it skip green without one: a + # canary that skips, is blocked (exit 3), never reaches the site (exit 5) + # or parses zero rows must end red, or the badge stops testing the site. + # If a proxy becomes required, the job should fail with an ::error:: + # naming AMAZON_PROXY when the secret is absent. variation-canary: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/weekly.yml b/.github/workflows/weekly.yml index c375d34..feba761 100644 --- a/.github/workflows/weekly.yml +++ b/.github/workflows/weekly.yml @@ -35,8 +35,9 @@ name: weekly # needs the workflow on the DEFAULT branch, or it answers a 404 that reads # like a filename typo), then read what actually happened per job. Add the # schedule below for the jobs that pass, and for any that are refused from -# runners either gate them on the AMAZON_PROXY secret with a ::notice:: skip -# — the pattern canary.yml's header describes — or drop them and say why. +# runners either require the AMAZON_PROXY secret (failing with an ::error:: +# that names it when it is absent, never a green skip) or drop them and say +# why. # # schedule: # - cron: "41 5 * * 1" # Mondays, off the hour