From 24caf56984abfccd13cfa246958e4d8265b780a2 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:29:04 +0000 Subject: [PATCH 1/2] Build the Docker image in CI, and pin claude.yml to the stable channel Two CI gaps, both of the same shape: a check that exists elsewhere in the family and never arrived here. No job built the Docker image. That is how three repos in this family shipped an image that died with ModuleNotFoundError on every invocation, --help included: the Dockerfile COPYs an explicit list -- correct, the image should carry no test suite and no stray .env -- and the list falls behind the imports with nothing to notice. The job is four steps, each for a distinct failure: the image builds; its entrypoint runs, which is the invocation a missing module breaks; Chromium actually launches, rather than the `playwright install --with-deps` layer merely exiting 0; and the image contains no .env, no test suite and no fixtures, because a .env baked into an image is a credential published to everyone who can pull it. The COPY list was verified against playwright_scraper.py's transitive local import graph before writing the job, and is complete. The job itself could not be run locally -- the docker daemon is not reachable from this account -- so its first real proof is this PR's own CI, which is the point of adding it. claude.yml let the action install `latest`. On 2026-09-08 that release installed no binary where the action looks, so every run died with "Claude Code native binary not found at ~/.local/bin/claude". The fix -- install the stable channel explicitly and pass path_to_claude_code_executable, with the path exported through GITHUB_ENV because ${{ env.HOME }} is empty in the workflow env context and the action then silently falls back to latest -- was already in claude-code-review.yml in this very repo and had never reached its twin. Pinning the CHANNEL rather than a version means a fixed upstream release needs no edit here. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/claude.yml | 23 +++++++++++++++++ .github/workflows/tests.yml | 49 ++++++++++++++++++++++++++++++++++++ CHANGELOG.md | 21 ++++++++++++++++ 3 files changed, 93 insertions(+) diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 15e0561..38d7d0f 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -42,11 +42,34 @@ jobs: with: fetch-depth: 1 + # Pin the CLI to the `stable` channel rather than letting the action + # install `latest`. On 2026-09-08 `latest` (2.1.265) installed no binary + # where the action looks, so every run died with "Claude Code native + # binary not found at ~/.local/bin/claude". `stable` is upstream's own + # known-good pointer, so this pins away from a broken release WITHOUT + # hardcoding a version: when upstream promotes a newer stable, this + # picks it up with no edit here. + # + # Remove both this step and path_to_claude_code_executable once `latest` + # installs correctly again. + - name: Install Claude Code (stable channel) + if: env.HAS_CLAUDE_TOKEN == 'true' + run: | + curl -fsSL https://claude.ai/install.sh | bash -s stable + "$HOME/.local/bin/claude" --version + # Through GITHUB_ENV, not `${{ env.HOME }}`: HOME is a runner + # variable and is not in the workflow `env` context, so the + # expression form expands to an empty string and the action falls + # back to installing `latest` — the very thing this step exists to + # avoid, and silently. + echo "CLAUDE_BIN=$HOME/.local/bin/claude" >> "$GITHUB_ENV" + - name: Run Claude Code id: claude if: env.HAS_CLAUDE_TOKEN == 'true' uses: anthropics/claude-code-action@v1 with: + path_to_claude_code_executable: ${{ env.CLAUDE_BIN }} claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} # This is an optional setting that allows Claude to read CI results on PRs diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 98c11e7..1baf261 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -208,3 +208,52 @@ jobs: - name: The ${{ matrix.engine }} CLI answers --help run: .venv/bin/python ${{ matrix.engine }}_scraper.py --help >/dev/null + + docker: + # Nothing else in this repo builds the image, which is how all three of + # the older repos in this family shipped one that died with + # ModuleNotFoundError on every invocation, `--help` included: the + # Dockerfile COPYs an explicit list (right -- the image should carry no + # test suite and no stray .env) and the list fell behind the imports. + # Four steps, each for a distinct failure. Under a minute. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Build the image + # Plain `docker build`, no registry and no push: the point is that the + # Dockerfile still builds, not that anything is published. + run: docker build -t amazon-scraper:ci . + + - name: The entrypoint runs inside the image + # Its CMD is --help, so a bare `run` exercises the default path. This + # is the invocation a missing module breaks. + run: docker run --rm amazon-scraper:ci + + - name: Chromium is actually installed in the image + # `playwright install --with-deps` is the layer this job exists to + # prove, so check the browser is there rather than trusting that the + # layer exited 0. + run: | + docker run --rm --entrypoint python3 amazon-scraper:ci -c " + from playwright.sync_api import sync_playwright + with sync_playwright() as p: + b = p.chromium.launch() + print('chromium', b.version) + b.close()" + + - name: The image carries no credentials and no test material + # It COPYs an explicit list precisely so that it does not. Verify + # rather than assume: a .env baked into an image is a credential + # published to everyone who can pull it. + run: | + set -e + found=0 + for f in .env smoke_test.py tests sample_output.json; do + if docker run --rm --entrypoint sh amazon-scraper:ci -c "test -e /app/$f"; then + echo "::error::$f is inside the image and should not be" + found=1 + fi + done + test "$found" = "0" + echo "no .env, no test suite, no fixtures in the image" diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a5276e..2f2176c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,27 @@ CLI toolkit can. Read a **patch** as "fixes" rather than as a promise that every flag and default is frozen: a default that changes behaviour can ship in one, and when it does the release notes say so first. +## [Unreleased] + +### CI + +- **The Docker image is now built in CI.** Nothing built it before, which is + exactly how three repos in this family shipped an image that died with + `ModuleNotFoundError` on every invocation, `--help` included — the + Dockerfile COPYs an explicit list, which is right, and the list fell behind + the imports. Four steps, each for a distinct failure: the image builds; its + entrypoint runs (the `CMD` is `--help`, the invocation a missing module + breaks); Chromium really launches, rather than the `playwright install + --with-deps` layer merely exiting 0; and the image contains no `.env`, no + test suite and no fixtures, because a `.env` baked into an image is a + credential published to everyone who can pull it. +- **`claude.yml` pins the CLI to the `stable` channel.** The fix had already + landed in `claude-code-review.yml` in this repo and never reached its twin, + so `claude.yml` still let the action install `latest` — the release that on + 2026-09-08 installed no binary where the action looks, failing every run + with "Claude Code native binary not found". The channel is pinned rather + than a version, so a fixed upstream release needs no edit here. + ## [0.1.3] — 2026-09-11 ### Fixed From 21757dd1d6f13955c55e357c24655a76ea966d45 Mon Sep 17 00:00:00 2001 From: jehrr <79864894+jehrr@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:37:52 +0000 Subject: [PATCH 2/2] Bind every call into a shared module against the callee's real signature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §17's check #1, which this repo did not have. It exists because of a defect no other check in a suite like this can see: a sibling shipped `classify(html, url=...)` in two of three engines against a callee whose second parameter is `status`, and BOTH crashed on their FIRST fetch -- invisible to import, --help, compileall, the undefined-name walk and several hundred green assertions, because none of those calls a function the way a live run does. Two failure modes, and the second is the one a weaker version swallows: a call whose arguments do not fit the signature, and a call to a name the shared module DOES NOT DEFINE. bbb-scraper resolved the callee with getattr(..., None) and skipped whatever came back not-callable, so three calls into an API that did not exist sat under a green run of its own binding check. Absent is the loudest failure available, not "nothing to bind", and it is reported here as a failure naming the caller and line. Conservative by construction: a call using *args or **kwargs is skipped rather than guessed at, and a name bound anywhere in the calling file shadows a same-named module -- an engine takes `proxy_pool` as a PARAMETER, and `proxy_pool.next()` on that parameter is a method call, not a module attribute. Without that rule the check reports twenty-one false positives on a clean repo. It also asserts that it found calls to bind at all, so it cannot pass by scanning nothing. Run across all eighteen repos of this family before being added to any of them: one real defect, in bbb-scraper, now fixed there. Verified by control -- adding a finish_run() call with thirteen positional arguments turns the suite red with "too many positional arguments", and removing it turns it green. The first version of this check crashed on that control instead of reporting it: it passed three arguments to a two-argument check(), which is precisely the defect class it exists to catch, found by its own control rather than by review. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 13 ++++++ smoke_test.py | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f2176c..814b0d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,19 @@ one, and when it does the release notes say so first. ## [Unreleased] +### Added + +- **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 - **The Docker image is now built in CI.** Nothing built it before, which is diff --git a/smoke_test.py b/smoke_test.py index fd787e6..2683e5f 100644 --- a/smoke_test.py +++ b/smoke_test.py @@ -35,6 +35,7 @@ """ import ast +import importlib import inspect import json import os @@ -1403,6 +1404,116 @@ def test_wording(): return ok +def test_shared_calls_bind_against_the_real_signature(): + """§17's check #1: 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 + see. A sibling repo shipped `classify(html, url=…)` in two of three + engines against a callee whose second parameter is `status`, and BOTH + crashed on their FIRST fetch — invisible to import, `--help`, + `compileall`, the undefined-name walk and several hundred green + assertions, because none of those calls a function the way a live run + does. + + Two failure modes, and the second is the one a weaker version of this + check swallows: a call whose arguments do not fit the signature, and a + call to a name the shared module **does not define at all**. Another + repo in this family resolved the callee with `getattr(..., None)` and + skipped whatever came back not-callable, so three calls into an API that + did not exist sat under a green run of its own binding check. Absent is + the loudest failure available, not "nothing to bind". + + Deliberately conservative: a call using `*args` or `**kwargs` is skipped + rather than guessed at, so this under-reports and never invents a + problem. + """ + group("every call into a shared module binds against its real signature") + ok = True + shared = {} + for name in ("product_parser", "output_writer", "page_flow", "proxy_pool", + "captcha_solver", "env_config", "scraper_api_client", + "fingerprint_client"): + if os.path.exists(os.path.join(REPO_ROOT, name + ".py")): + try: + shared[name] = importlib.import_module(name) + except Exception as exc: # noqa: BLE001 + ok &= check("%s imports (%r)" % (name, exc), False) + + callers = [e + ".py" for e in ENGINES] + callers += [n for n in ("page_flow.py", "product_parser.py", + "scraper_api_client.py") + if os.path.exists(os.path.join(REPO_ROOT, n))] + checked = 0 + for filename in callers: + path = os.path.join(REPO_ROOT, filename) + if not os.path.exists(path): + continue + tree = ast.parse(open(path, encoding="utf-8").read()) + + # Names bound anywhere in this file shadow a same-named module: an + # engine takes `proxy_pool` as a PARAMETER, and `proxy_pool.next()` + # on that parameter is a method call, not a module attribute. + bound = set() + for node in ast.walk(tree): + if isinstance(node, ast.Name) and isinstance(node.ctx, (ast.Store, ast.Del)): + bound.add(node.id) + elif isinstance(node, ast.arg): + bound.add(node.arg) + elif isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + bound.add(node.name) + + direct = {} + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom) and node.module in shared and not node.level: + for alias in node.names: + direct[alias.asname or alias.name] = (node.module, alias.name) + + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + target = None + if isinstance(node.func, ast.Name) and node.func.id in direct: + target = direct[node.func.id] + elif (isinstance(node.func, ast.Attribute) + and isinstance(node.func.value, ast.Name) + and node.func.value.id in shared + and node.func.value.id not in bound): + target = (node.func.value.id, node.func.attr) + if target is None: + continue + module_name, attr = target + module = shared[module_name] + if not hasattr(module, attr): + ok &= check("%s:%d calls %s.%s, which does not exist — a " + "live run reaches this as AttributeError" + % (filename, node.lineno, module_name, attr), + False) + continue + callee = getattr(module, attr) + if not (inspect.isfunction(callee) or inspect.isclass(callee)): + continue + if (any(isinstance(a, ast.Starred) for a in node.args) + or any(k.arg is None for k in node.keywords)): + continue # unpacking: cannot bind statically + try: + signature = inspect.signature(callee) + except (TypeError, ValueError): + continue + try: + signature.bind(*([inspect.Parameter.empty] * len(node.args)), + **{k.arg: inspect.Parameter.empty + for k in node.keywords}) + except TypeError as exc: + ok &= check("%s:%d %s.%s%s — %s" + % (filename, node.lineno, module_name, attr, + signature, exc), False) + else: + checked += 1 + ok &= check("...and there were calls to bind (%d)" % checked, checked > 10) + return ok + + def test_sample_output(): group("shipped sample output") ok = True @@ -1556,6 +1667,7 @@ def main() -> int: ok &= test_ci_checks_is_actually_wired_up() ok &= test_fingerprint_client_reads_env() ok &= test_wording() + ok &= test_shared_calls_bind_against_the_real_signature() ok &= test_sample_output() print()