Repository navigation
Build the Docker image in CI, and pin claude.yml to the stable channel - #13
Conversation
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) <noreply@anthropic.com>
| - 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" |
There was a problem hiding this comment.
Altitude / reuse: this duplicates claude-code-review.yml verbatim, reproducing the exact drift risk this PR's own CHANGELOG describes.
This "Install Claude Code (stable channel)" block (and the matching path_to_claude_code_executable: ${{ env.CLAUDE_BIN }} line below) is a line-for-line copy of the block already in .github/workflows/claude-code-review.yml. The CHANGELOG entry for this PR even says so explicitly: "The fix had already landed in claude-code-review.yml in this repo and never reached its twin."
Copy-pasting the fix into the second file doesn't remove the underlying problem — it just resets the clock on it. There's no .github/actions/ composite action in this repo, so the next time this install step needs to change (upstream deprecates stable, the install path changes, etc.), whoever edits one workflow can just as easily forget the other again, exactly as happened here.
Suggest factoring this into a composite action (e.g. .github/actions/install-claude-stable/action.yml) that both claude.yml and claude-code-review.yml call, so there is exactly one place to fix next time instead of two files that can silently drift apart.
§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) <noreply@anthropic.com>
| # 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. |
There was a problem hiding this comment.
"Under a minute" reads as a measured number but nothing here measures it, and the job has no docker layer cache or actions/cache for pip/playwright (no cache-from, no actions/cache, no BuildKit cache mount). A cold docker build that runs pip install plus playwright install --with-deps chromium — apt-installing browser dependencies and downloading the Chromium binary — routinely takes well over a minute on a fresh ubuntu-latest runner, before the three docker run steps that follow. Either cite an actual observed runtime (and note it's cold-cache) or drop the specific figure so the comment doesn't read as a benchmarked fact.
| 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))] |
There was a problem hiding this comment.
callers omits fingerprint_client.py, even though it's listed in shared (line 1436) and does make real calls into another shared module — env_config.load_env() and env_config.env_value("TWOCAPTCHA_KEY") at fingerprint_client.py:348-349. Those calls are structurally unreachable by this test's ast.walk loop.
That undercuts the claim this check makes in CHANGELOG.md (line 15): "binds every call into a shared module against the callee's real signature." It's not every call — calls made from one shared module into another are skipped entirely, and this is exactly the file/call-path (fingerprint_client.py → env_config) that CHANGELOG's [0.1.3] entry says had a real prior defect. If env_config.env_value's signature ever changes in a way that breaks fingerprint_client.py's call, this test stays green while a live run crashes on first use.
Suggest adding "fingerprint_client.py" to callers.
| if isinstance(node.func, ast.Name) and node.func.id in direct: | ||
| target = direct[node.func.id] |
There was a problem hiding this comment.
Minor asymmetry: this direct-import branch resolves a call by bare name alone, with no check against bound. The ast.Attribute branch just below (lines 1478-1482) explicitly excludes names in bound so a locally shadowed module reference isn't mistaken for the real module — this branch has no equivalent guard. If a caller ever reassigns a directly-imported name (e.g. parse_products = some_local_wrapper then calls parse_products(...)), this would bind the call against product_parser.parse_products's real signature instead of the local rebinding, producing a false pass/fail unrelated to what's actually being called at runtime.
Not triggered by any current caller, but worth the same and node.func.id not in bound guard for consistency with the deliberately-conservative design described in the docstring above.
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-depslayer 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 releaseinstalled 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) noreply@anthropic.com
🤖 Generated with Claude Code