@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

Claude finished @axisrow's task in 3m 49s —— View job

PR Review — Critical Issues Only

• Read PR diff against origin/main
• Review captcha guards in fetchers
• Review check_all_docs_urls.py script
• Review release_pypi.sh pre-release gate changes
• Review cache-content tests
• Post review findings

Solid PR overall — the captcha guards, cache-content tests, and URL drift checker are the right shape. Below are the issues I'd want addressed before this becomes the gating release path. Everything else (style, naming, thresholds) I'm intentionally skipping per the review brief.

1. HEAD-only probing will produce false GONE failures and block releases

File: scripts/check_all_docs_urls.py:53-72

The probe issues requests.head(...) first and only falls through to GET on a 2xx. If Yandex's CDN ever responds 405 (Method Not Allowed) or 403 to HEAD for any of the 37 URLs, the script classifies it as GONE (4xx → hard fail) and release_pypi.sh aborts the release. CDNs frequently treat HEAD differently from GET — and Yandex's own Cloudflare-style fronting is known to do this for some doc routes.

This is the same class of false-positive the PR is trying to prevent (a working page that looks broken), just moved one layer up. Recommend: on HEAD 4xx, retry with GET before declaring GONE. Or use GET with stream=True from the start and just read headers.

Fix this →

2. Pre-release gate has no escape hatch and runs network checks on every release

File: scripts/release_pypi.sh:72-83, 119

prerelease_docs_health_check is mandatory and runs before build_artifacts. The PR description proudly notes "No --skip-checks flag" — which is the right default, but combined with:

• 37 URLs × 1.5s pacing = ~55s minimum
• Single 30s captcha retry per URL
• Any single CAPTCHA / SMALL / GONE / ERROR / MOVED hard-fails
• HEAD-405 false positives (see Add Claude Code GitHub Workflow #1)

…this means a captcha sweep from Yandex during the release window blocks PyPI publishing with no override. For a hotfix release that doesn't touch any docs URLs, this is a real risk. Worth at least an explicit env-var escape (e.g. SKIP_DOCS_HEALTH_CHECK=1) gated behind a clear warning, even if it's not a CLI flag.

3. refresh_reports_cache.py mutates committed files during release

File: scripts/release_pypi.sh:79

python3 scripts/refresh_reports_cache.py writes to tests/reports_cache/raw/*.html and tests/reports_cache/spec.json. If Yandex updated any doc page since the last commit, the release proceeds with a dirty working tree, and the maintainer ends up either (a) committing post-release fixture drift, or (b) silently dropping the new content. The release script doesn't git diff --exit-code or fail if the cache was modified.

Suggest either:

• Run with use_cache=True and fail if the live content differs from the committed cache (force a separate commit before release), or
• Move the refresh out of the release path entirely and rely on the existing scheduled drift monitor.

4. Captcha protection does not cover imported XSDs

File: direct_cli/wsdl_coverage.py:377-396 and tests/test_api_coverage.py:2280-2308

_assert_real_wsdl is only called from fetch_wsdl and fetch_live_wsdl. fetch_imported_xsd (line 377) does the same requests.get(...).text → cache_file.write_text(...) dance with no validation, so an imported XSD can still be silently captcha-poisoned. This is the exact failure mode the PR is closing for WSDLs; the imports path has it open.

The new test_wsdl_cache_files_are_real_content test also uses cache.glob("*.xml") which is non-recursive, so files in tests/wsdl_cache/imports/ are not checked.

Recommend: add a sibling _assert_real_xsd (same captcha markers, lower min-size threshold, require xsd:schema instead of wsdl:definitions) and call it from fetch_imported_xsd; extend the cache-content test to cache.rglob("*.xml") or add a second loop over imports/.

Fix this →

5. _assert_real_wsdl substring match is namespace-prefix-dependent

File: direct_cli/wsdl_coverage.py:164-184

_WSDL_REQUIRED_MARKERS = ("<?xml", "wsdl:definitions") is a raw substring check on the XML text. If Yandex ever changes the namespace prefix (e.g., to <definitions xmlns:wsdl="..."> with definitions as the root element rather than wsdl:definitions, which is perfectly valid WSDL), this will reject real responses and brick the cache refresh.

Today this is fine — all current Yandex WSDLs use wsdl: prefix — but it's a known-fragile guard. Minor, but worth a comment noting the assumption or a regex that allows any namespace prefix on definitions.

Not flagged (intentionally, per the brief)

• URL fixes in RESOURCE_MAPPING_V5 and refreshed HTML fixtures look correct and match the canonical paths.
• The strict body-size / captcha-marker thresholds are arbitrary but defensible.
• CHANGELOG, CLAUDE.md, test additions all look consistent.

Summary

Items 1 and 2 together are the most likely to cause real production pain (a transient Yandex anti-bot event blocking a hotfix). Item 4 is a genuine coverage gap in the captcha protection. Items 3 and 5 are smaller but worth addressing.
· fix/426-reports-drift-urls-and-protection

Triage applied in 1f1b8d3:

• Add Claude Code GitHub Workflow #1 (HEAD-only → GONE): FIX. _probe now only decides GONE after a GET, not after a HEAD 4xx — CDN false positives no longer block release.
• feat: add Bitwarden CLI integration #3 (refresh mutates working tree): FIX. Removed refresh_reports_cache.py from the release path. The gate now runs the read-only freshness tests and refuses to release if tests/reports_cache/tests/wsdl_cache is dirty.
• Fix all linting and formatting issues #4 (imported XSDs unprotected): FIX. Both sub-claims verified — fetch_imported_xsd had no validation, and the test used glob('*.xml') (non-recursive, wrong extension). Added _assert_real_xsd + a sibling test_imported_xsd_cache_files_are_real_content covering imports/*.xsd.
• Add 1Password auth and simplify build config #2 (no escape hatch): SKIP. This was a deliberate design choice per the project's "no opt-out gates" policy (codified in CLAUDE.md). With Add Claude Code GitHub Workflow #1 fixed, transient Yandex anti-bot events no longer cascade into a hard fail — the legitimate concern behind Add 1Password auth and simplify build config #2 dissolves into the same path Add Claude Code GitHub Workflow #1 was already taking.
• [direct-cli] Расширить CLI на все 25 сервисов Яндекс.Директ API v5 #5 (wsdl:definitions substring is prefix-dependent): SKIP. You concede "Today this is fine — minor" and Yandex has used wsdl: consistently across all 29 cached WSDLs. Will revisit if/when Yandex changes the prefix; not worth pre-emptive regex complexity today.

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

Claude finished @axisrow's task in 2m 5s —— View job

Re-review — follow-up commit 1f1b8d3

• Diff against last review
• Verify fix for Add Claude Code GitHub Workflow #1 (HEAD → GET fallback before GONE)
• Verify fix for feat: add Bitwarden CLI integration #3 (refresh removed; dirty-tree check)
• Verify fix for Fix all linting and formatting issues #4 (imported XSD validation + recursive test)
• Look for new issues introduced by the fixes
• Post review

All three FIX items look correctly implemented. Notes below are not blockers, just observations from re-reading the patch.

#1 — HEAD→GET fallback (FIXED, correctly)

scripts/check_all_docs_urls.py:65-97 — HEAD now only short-circuits to MOVED on a 3xx-with-different-path or SERVER on 5xx; everything else (including 403/405 from a CDN that doesn't like HEAD) falls through to GET, and GONE is decided from the GET status. The error string preserves both codes for diagnosis ("HTTP 404 (HEAD was 405)"). The captcha-redirect retry on HEAD is preserved.

One minor edge case worth knowing about (not a blocker): the GET on line 79 doesn't set allow_redirects=False, so if the CDN rejects HEAD with 4xx and the GET would 301 to a different canonical path, requests follows the redirect silently and we mark it OK. This means the MOVED-detection only fires when HEAD returns a redirect — for the rare URL that 403s HEAD but redirects on GET, a canonical-path migration would be missed until a subsequent HEAD probe sees the redirect directly. Won't cause a bad release; just a small gap in drift detection on that specific path.

#3 — refresh out of release path (FIXED, with one subtlety)

scripts/release_pypi.sh:80-94 no longer calls refresh_reports_cache.py, runs only the read-only freshness pytest, and then git diff --quiet -- tests/reports_cache tests/wsdl_cache blocks release on a dirty tree. This is the right shape.

One subtle behavior worth flagging (borderline minor): git diff --quiet -- <paths> compares working tree against the index, not against HEAD. So this sequence:

python scripts/refresh_reports_cache.py
git add tests/reports_cache tests/wsdl_cache
./scripts/release_pypi.sh all

…would pass the gate even though the cache has uncommitted-to-HEAD changes. The intent of the gate ("maintainer must commit the refresh explicitly") is better expressed as:

if ! git diff --quiet HEAD -- tests/reports_cache tests/wsdl_cache; then

That covers both unstaged and staged-but-uncommitted changes. As-is, the practical risk is small (released wheels don't include tests/), but it's a one-word change that fully matches the documented intent.

#4 — imported XSD captcha guard + recursive test (FIXED, correctly)

• direct_cli/wsdl_coverage.py:237-258 — _assert_real_xsd checks both captcha markers, min-size (1 KB, calibrated for the smallest current import), the XML declaration, and a schema substring. Called from fetch_imported_xsd on line 420 right after raise_for_status() and before the cache write — that's the correct ordering (validate before persist).
• tests/test_api_coverage.py:2318-2333 — new test_imported_xsd_cache_files_are_real_content globs imports/*.xsd (correct extension this time, matching the actual on-disk files) and applies the same four invariants. Together with the existing WSDL test, both halves of the cache are now guarded.

Implementation matches the production guard's checks one-for-one.

Verdict

The triage applied in 1f1b8d3 addresses the three FIX items I flagged. The two SKIP items (#2 escape hatch, #5 namespace prefix) are defensible per the author's reasoning — #2's underlying risk dissolves now that #1 no longer cascades HEAD failures into release blocks, and #5 is a documented-as-fragile guard with no observed drift.

The two minor notes above (GET redirect handling in _probe, git diff vs git diff HEAD) are not critical-tier — neither can cause a bad release or data loss in normal maintainer flow. Mentioning them only because the user asked for a focused critical review and these are the only loose threads I can find in the follow-up; entirely OK to defer or close as won't-fix.

LGTM to merge as-is.
· fix/426-reports-drift-urls-and-protection