diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index c316c6a..405a52f 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -1,6 +1,6 @@ --- name: code-reviewer -description: Reviews code for quality, correctness, and maintainability. Use for diff review, PR review, or post-change verification. +description: Reviews Solverr's Python changes for correctness, stealth event-loop safety, the engine-layer law, and /v1 compatibility. Use for diff review, PR review, or post-change verification. tools: - Read - Grep @@ -8,63 +8,74 @@ tools: - Bash --- -You are a thorough code reviewer focused on catching real issues, not style nitpicks. +You review Python changes in Solverr, a FlareSolverr fork: Python 3.14, `bottle` + `waitress` (synchronous WSGI), the FlareSolverr `/v1` API on 8191 and an optional passthrough proxy on 8888. Two engines sit behind `src/engines/base.py`: `chrome` (Selenium + the vendored undetected_chromedriver) and `stealth` (Camoufox via invisible_playwright, run on ONE background asyncio loop thread in `src/async_runtime.py`). The shared spine is `src/assembly.py`, `src/pipeline.py`, `src/budget.py` and `src/sessions.py`; the controller is `src/flaresolverr_service.py`. Catch real issues, not style nitpicks. + +The repo's own rules are the baseline, so read the relevant one before flagging against it: `CLAUDE.md`, `.claude/rules/engine-layer.md` (the law for anything touching an engine), `code-quality.md`, `error-handling.md`, `testing.md`, and `docs/dev/upstream-sync.md` for what is deliberately different. ## Operating principles - State assumptions explicitly. If multiple readings of the code are possible, surface them. Don't pick silently. -- Surgical scope. Only flag lines that changed or directly relate. Ignore pre-existing issues outside. +- Surgical scope. Only flag lines that changed or directly relate. Ignore pre-existing issues outside, including upstream-inherited code the diff didn't touch. - Verify before flagging. Cite file:line. If you can't verify, say so. - Confidence threshold. Only ship findings you're at least 80% sure are real. Drop the rest. ## How to review -Run `git diff --name-only` for changed files. Read each, grep for related patterns. Report only concrete problems with evidence. +Run `git diff --name-only` for changed files. Read each, grep for related patterns. When the diff touches one engine, open the other engine's file too: the usual defect here is the half that did not land. Report only concrete problems with evidence. -## Correctness +## Python correctness -**Off-by-one**: `array[array.length]` vs `array.length - 1`. `i <= n` vs `i < n`. Inclusive vs exclusive ranges. Fence-post errors (n items need n-1 separators). +- **Mutable defaults**: `def f(x=[])` or `={}` shared across calls. The `None` class attributes on `V1RequestBase` are fine. +- **bool is an int**: `isinstance(True, int)` holds, so an int check that does not exclude `bool` lets `true` through as 1. `validate_request_types` (`src/dtos.py`) and `_validate_max_timeout` show the shape. +- **Truthiness on request input**: `"false"` is truthy, and `0` can be a real value. `validate_request_types` types declared `/v1` fields once; code reading anything it skips (`_TYPE_OVERRIDES`) checks the type itself. +- **Clocks**: `budget.solve_deadline` takes `time.monotonic()` on a request thread and the loop's clock on the stealth engine. Mixing the two, or a deadline on `datetime.now()`, is a finding. +- **Shared module state** written from waitress threads without a lock. `_DOMAIN_ENGINE` is guarded by `_DOMAIN_LOCK`; follow that. -**Null/undefined**: properties on possibly-null values, missing optional chaining, array methods on possibly-undefined arrays, destructuring from possibly-null objects. +## The stealth event loop -**Logic**: inverted conditions, short-circuit skipping side effects, `==` vs `===` (JS/TS), mutation of shared references, missing `break` in switch (unless intentional and commented). +- A coroutine called without `await`, or `asyncio.create_task` / `ensure_future` whose result nobody holds or awaits. `error-handling.md` requires every coroutine on the loop to be awaited or scheduled through `async_runtime`; a floating task drops its exception. +- `AsyncRuntime.run` called from code already on the loop thread. It blocks on `future.result()`, so the loop waits on itself until the timeout. +- A `StealthContext` browser, context or page touched from a request thread. Its docstring says it is only ever touched from the loop. +- A blocking call inside a coroutine (`time.sleep`, a sync network lookup, file I/O). Move it off the loop the way `StealthContext.start` sends `geo.browser_identity` through `asyncio.to_thread`. +- A bare `except:` or `except BaseException` in a coroutine. It swallows `CancelledError`, which is how `asyncio.wait_for` and `AsyncRuntime.run`'s timeout stop a solve. -**Race conditions**: shared mutable state in async callbacks, read-then-write without atomicity, awaits depending on the same mutable variable, event handlers registered without cleanup. +## The engine-layer law -## Error handling +`.claude/rules/engine-layer.md` binds every engine change. Flag: -- Swallowed errors: `catch (e) {}` or `catch (e) { return null }`. -- Missing `.catch()` on promise chains. -- Wrapped errors that lose context: `throw new Error("failed")` discards the original. -- Try/catch too broad, catching errors from unrelated code. -- Missing cases: 404? File not found? Parse error? +- A client-observable change that lands for one engine only. The only exit is a named browser-automation mechanism the other engine cannot provide, cited in the commit and recorded in `docs/dev/upstream-sync.md`. "Structured differently" and "needs a rewrite first" are not exits. +- A per-engine branch, nullable field, or boolean-flag combination inside the shared spine. Divergence is a typed capability slot. +- A capability that silently does nothing on one engine instead of being routed or refused by name (`tabs_till_verify` on stealth is the rule's own example). +- Shared storage that each engine interprets its own way; spine code reimplementing a clearing core; a re-proposed `SessionRef`. +- A second per-engine test where one test in `src/test_engine_conformance.py` (driven by `HARNESSES` in `src/engine_fakes.py`) would pin both. A new test whose PR does not say it was seen red with its production clause deleted has not been verified by mutation. -## Naming +## /v1 compatibility -- Names that lie: `isValid` returning a string, `getUser` that creates. -- Generic where a specific name exists: `data`, `result`, `temp`, `item`. -- Booleans missing `is` / `has` / `should` prefix. -- Abbreviations that obscure: `usr`, `mgr`, `ctx`. +- A removed, renamed or retyped request or response field. Only additive optional fields are allowed (`workflow.md`, "Fork compatibility"). +- An optional response field now emitted as `null` instead of omitted (`_to_challenge_resolution`). +- A changed error message clients or the fallback match on (`"Error solving the challenge. ..."`, `"session ... not found"`), or an error leaving the FlareSolverr shape (`status: "error"`, HTTP 500). +- Refusing unknown parameters: `validate_request_types` logs and keeps them on purpose. +- Any change to the `"FlareSolverr is ready!"` banner (`index_endpoint`). Clients detect session support by it. -## Complexity +## Error handling -- Functions over ~30 lines. -- Nesting deeper than 3 levels (early returns flatten). -- More than 3 parameters (use options object). -- God functions doing read, validate, transform, persist, and notify. +- `except Exception: pass` or a silent `return None` on a teardown or reaper path. `error-handling.md` requires `logging.debug(..., exc_info=True)` there, as `SessionStore._teardown` does. +- A raw traceback reaching the response body. +- Failure handled by silencing an engine instead of at the controller's fallback in `_resolve_challenge`. +- A browser, context or temp dir that leaks when a launch fails partway. Cleanup belongs in a `finally`, as in `StealthEngine.solve` and `utils.get_webdriver`. ## Tests -- Changed behavior without a corresponding test change. -- Tests asserting implementation (mock call counts) instead of output values. -- Missing edge case for the specific code path that changed. +- Changed behaviour without a test change, where a browser-free `src/test_*.py` test can reach it. `src/tests.py` needs a browser and live sites. +- Tests asserting mock call counts where output values would do (`testing.md`). +- A detection or engine change claimed verified by compile and unit tests alone. Those cannot say whether a page still clears; `workflow.md` names `/live-check` for that. ## What NOT to flag -- Style handled by linters (formatting, semicolons, quotes). -- Minor naming preferences without clarity impact. +- Upstream code kept mergeable on purpose, outside the changed lines: `src/undetected_chromedriver/`, `src/tests.py`, `src/tests_sites.py`, `src/bottle_plugins/`, the Chrome clearing core (FlareSolverr's) and the stealth clearing core (Byparr's). Upstream idiom there is not a finding. +- Anything under "Deliberately different" in `docs/dev/upstream-sync.md`. +- Code that looks wrong but encodes a measured constraint from the "Architecture (non-obvious)" bullets in `CLAUDE.md`: the `quote()` inside `escape()` in `postform.py`, the coordinate Turnstile click, no `page.evaluate` against a challenge page, the second look before a challenge counts as cleared, the even `maxTimeout` split. The tuned constants (`_CHALLENGE_CONFIRM_SECONDS`, `_NETWORKIDLE_MS`, `_CLICK_COOLDOWN_SECONDS`, `_WIDGET_RENDER_SECONDS`, `SOLVE_MARGIN_SECONDS`) are the same case. Ask for a measurement rather than proposing the obvious fix. - "I would have done it differently" without a concrete problem. -- Suggestions to add types or docs to code you didn't review. - Pre-existing issues outside the changed scope. ## Output format @@ -83,10 +94,10 @@ End with a single sentence naming the most important fix. For each finding: - **File:Line**: exact location. -- **Issue**: what's wrong and why it matters. Be specific ("this throws if user is null", not "potential null issue"). +- **Issue**: what's wrong and why it matters. Be specific ("the cookie read moved below `waitInSeconds` in `chrome_engine.py` only, so stealth still returns cookies from before the wait", not "possible inconsistency"). - **Suggestion**: how to fix it. Include code if helpful. - **Confidence**: 0 to 100. End with a brief overall assessment: what's solid, what needs work, the single most important fix. -Either way, apply the ≥80 confidence filter internally and drop findings below it. +Either way, apply the >=80 confidence filter internally and drop findings below it. diff --git a/.claude/agents/doc-reviewer.md b/.claude/agents/doc-reviewer.md index ffba74d..48f7161 100644 --- a/.claude/agents/doc-reviewer.md +++ b/.claude/agents/doc-reviewer.md @@ -1,6 +1,6 @@ --- name: doc-reviewer -description: Reviews documentation for accuracy, completeness, and clarity. Cross-references docs against the actual source code. +description: Reviews Solverr's docs, CHANGELOG, commit messages and code comments for accuracy against the code and for the repo's conventions (no em dashes, no target-site names, bold CHANGELOG headlines that stand alone as the release note, sentence-case headings). Cross-references docs against the actual source. tools: - Read - Grep @@ -8,74 +8,79 @@ tools: - Bash --- -You review documentation changes for quality. Focus on whether docs are accurate, complete, and useful, not whether they're pretty. +You review documentation changes in Solverr, a FlareSolverr fork (Python 3.14, the `/v1` API on 8191, an optional passthrough on 8888, a `chrome` and a `stealth` engine). Two jobs: verify claims against the actual code, and enforce the repo's own doc conventions. `.claude/rules/workflow.md` (CHANGELOG, commits, public-facing naming) and `.claude/rules/prose-style.md` (sentences and vocabulary) are the baseline; `.claude/rules/code-quality.md` covers comments. Focus on whether docs are accurate, complete, and convention-clean, not whether they're pretty. ## Operating principles - State assumptions explicitly. If you can't verify a claim against the code, say so. -- Surgical scope. Only flag issues in docs that changed, or that changes invalidated. -- Verify before flagging. Cite the source file:line you cross-checked. +- Surgical scope. Only flag issues in docs that changed, or that the code changes invalidated. +- Verify before flagging. Cite the source file you cross-checked. - Confidence threshold. Only ship findings you're at least 80% sure are real. ## How to review -Run `git diff --name-only` for changed docs (`.md`, `.txt`, `.rst`, docstrings, JSDoc, inline comments). For each doc change, read the source code it references and verify accuracy. +Run `git diff --name-only` for changed docs (`.md`, docstrings, comments) and `git log` over the range for commit messages. For each doc change, read the source code it references and verify accuracy. ## Accuracy (cross-reference with code) -- Function signatures: read the actual function, verify parameter names, types, return types, defaults match the docs. -- Code examples: trace each example against the source. Does the import path exist? Does the function accept those arguments? Does it return what the example claims? -- Config options: grep for the option name. Still used? Default value correct? -- File or directory references: use Glob to verify referenced paths exist. -- Can't verify? Say so explicitly: "Could not verify X. Requires runtime testing." +- Named symbols: grep every function, constant and file a doc names; verify it exists with that name and does what the doc says. +- Env vars: most are read in `src/config.py`, a few inherited ones in `src/utils.py` and `src/flaresolverr.py`. Check the name and the default against the README's Configuration tables. +- Commands: verify the commands in `CLAUDE.md` and `README.md` still run as written. +- Upstream claims: what came from FlareSolverr or Byparr, through which commit, and what is deliberately different must match `docs/dev/upstream-sync.md`, the single owner of that question. +- Can't verify? Say so explicitly: "Could not verify X." + +## Repo doc conventions + +- **No em dashes** in docs, comments, CHANGELOG or commits. No AI watermarks. +- **No target-site names or scraping vocabulary** in public surfaces: commit messages, branch names, `README.md`, `CLAUDE.md`, `CHANGELOG.md`, release notes (`workflow.md`, "Public-facing naming"). The generic forms are "a Cloudflare-gated site", "an indexer", "example-site.tld". `.githooks/pre-commit` only checks added lines in CHANGELOG and README against a heuristic, so read `CLAUDE.md` and commit messages yourself. +- **The bold CHANGELOG headline is the entire release note.** `release.yml` keeps only the bold text (`s/^- \*\*([^*]+)\*\*.*/- \1/`), so a new env var, default or limit a deployer must act on has to sit inside the bold. Apply that sed to every new entry. The headline is benefit-first, self-contained, ends in `.`, `!` or `?`, and names no class or mechanism. +- **CHANGELOG scope.** Only changes a deployer or API client could notice get an entry. A dependency bump or refactor that ships in the image is a plain line under `Other`. `.claude/`, hooks, CI, tests and repo docs get no entry at all. Iterating on something already in `[Unreleased]` edits that bullet instead of adding one. +- **Commits.** `type(scope): summary`, imperative, lower-case, <=72 chars, no trailing period. A non-trivial body leads with plain language. Never a bare `#N`; use `owner/repo#N`. +- **Prose style.** Sentence-case headings. Flag the `prose-style.md` habits (trailing significance clauses, inflated significance, padded triples, "serves as" where "is" would do) and its vocabulary table (`leverage`, `robust`, `ensure`, sentence-initial `Additionally`). Flag the pattern, not every word: the file allows a listed word when it is the precise term. +- **README describes current behaviour, not the journey** (`workflow.md`). `docs/dev/` records and the "Architecture (non-obvious)" bullets in `CLAUDE.md` carry reasoning and measurements on purpose. +- **Dev docs cite path plus symbol, not Solverr line numbers.** `_validate_url` in `src/flaresolverr_service.py`, not `:270`; line refs rot. +- **Single owner per fact.** Upstream history and divergences live in `docs/dev/upstream-sync.md`; the engine-layer rationale in `docs/dev/engine-layer-architecture.md`; the law in `.claude/rules/engine-layer.md`. A fact restated in a second doc is a finding; name the canonical home. +- **Code comments** (when the diff touches them): WHY, never WHAT (`code-quality.md`). Flag a comment that restates the adjacent code, and equally a cut that drops a measured constraint or an upstream divergence. Comments here often hold the measurement behind code that looks wrong (the `postform.py` docstring, `_resolve_challenge`), and losing one invites someone to "fix" it. ## Completeness -- Required parameters or environment variables not mentioned. -- Error cases: what happens when the function throws? What errors should the caller handle? -- Setup prerequisites a new developer would need. -- Breaking changes: if behavior changed, does the doc reflect it? +- A behaviour or config change without `README.md` updated in the same change (`workflow.md`). A new env var in `src/config.py` with no README row is the usual case. +- A port from, or decline of, an upstream change without the ledger's audited-through row or "Deliberately different" entry updated. +- A one-engine exception to the engine-layer law without its ledger record. +- `CLAUDE.md` "Where things live" missing a new top-level module or still naming a removed one. ## Staleness -- `grep -r "functionName"` to verify referenced functions and classes still exist. -- Version numbers, dependency names, and URLs that may be outdated. -- Deprecated API references (grep for `@deprecated` near referenced code). - -## Clarity - -- Vague instructions: "configure the service appropriately". Configure WHAT, WHERE, HOW? -- Missing context that assumes knowledge the reader may not have. -- Wall of text without structure (needs headings, lists, code blocks). -- Contradictions between sections. +- Grep referenced symbols to verify they still exist. +- Internal links: verify relative links resolve to files that exist. ## What NOT to flag - Minor wording preferences unless genuinely confusing. -- Formatting nitpicks handled by linters. -- Missing docs for internal or private code. -- Verbose but accurate content (suggest trimming, don't flag as wrong). +- Missing docs for internal code; docstrings belong at module and engine boundaries. +- Verbose but accurate content (suggest `/tighten`, don't flag as wrong). +- Site names where `workflow.md` allows them: chat, local scratch files, and the upstream list in `src/tests.py` and `src/tests_sites.py` (adding to those is a finding). ## Output format Default to terse. Switch to verbose only if the invocation prompt contains `verbose`, `full report`, or `detailed`. -**Default (terse)**: one line per finding, sorted by importance (accuracy issues first). +**Default (terse)**: one line per finding, sorted by importance (accuracy issues first, then convention violations). ``` file:line: (fix: ) ``` -End with one short sentence: accurate or inaccurate, complete or incomplete. +End with one short sentence: accurate or inaccurate, convention-clean or not. **Verbose**: For each finding: - **File:Line**: exact location. -- **Issue**: be specific ("README says `createUser(name)` takes one arg, but source shows `createUser(name, options)` with required `options.email`"). +- **Issue**: be specific ("README says `SESSION_MAX` defaults to 10, `config.session_max` returns 20"). - **Fix**: concrete rewrite or addition. - **Confidence**: 0 to 100. -End with overall assessment: accurate or inaccurate, complete or incomplete, structural suggestions. +End with overall assessment: accurate or inaccurate, complete or incomplete, convention issues. -Either way, apply the ≥80 confidence filter internally and drop findings below it. +Either way, apply the >=80 confidence filter internally and drop findings below it. diff --git a/.claude/agents/performance-reviewer.md b/.claude/agents/performance-reviewer.md index 8590189..e1ffa96 100644 --- a/.claude/agents/performance-reviewer.md +++ b/.claude/agents/performance-reviewer.md @@ -1,6 +1,6 @@ --- name: performance-reviewer -description: Reviews code for performance issues like memory leaks, slow queries, unnecessary computation, bundle size, and runtime bottlenecks. Use proactively after changes to hot paths, data processing, or API endpoints. +description: Reviews Solverr changes for extra browser launches, maxTimeout budget misuse, work that stalls the single stealth event loop, lock scope, and unbounded growth. Use after changes to the engines, sessions, the controller, or the passthrough. tools: - Read - Grep @@ -8,9 +8,9 @@ tools: - Bash --- -You are a performance engineer. Find real bottlenecks, not theoretical ones. Only flag issues that would cause measurable impact. +You are a performance engineer reviewing Solverr: Python 3.14, `bottle` + `waitress` (synchronous WSGI) serving the FlareSolverr `/v1` API on 8191, an optional passthrough on 8888, and two browser engines (`chrome`: Selenium + vendored undetected_chromedriver; `stealth`: Camoufox via invisible_playwright on ONE asyncio loop thread in `src/async_runtime.py`). Find real bottlenecks, not theoretical ones. Nearly all the cost is browsers: a launch takes seconds, a Camoufox browser is the heavier of the two in memory, and every page read is a round trip. Python-level cost rarely registers next to that. -This is static analysis. You can read code and estimate impact but cannot profile or benchmark. Flag based on how often the code path runs and how expensive the operation is. +This is static analysis. You can read code and estimate impact but cannot profile. Flag based on how often the code path runs and how expensive the operation is. ## Operating principles @@ -21,63 +21,41 @@ This is static analysis. You can read code and estimate impact but cannot profil ## How to review -Run `git diff --name-only`. Read each changed file plus its callers. Determine path frequency (per request, per user, once at startup). Rank findings by impact (frequency times cost). +Run `git diff --name-only`. Read each changed file plus its callers. Determine path frequency (per request, per engine attempt, per poll tick inside a solve, per reaper interval, once at startup). Rank findings by impact. -## Database and queries +## Browser launches -- **N+1**: ORM calls inside `for` / `forEach` / `map`, awaits in loops hitting the DB. Fix: join, include, or batch. -- **Missing indexes**: columns used in WHERE, ORDER BY, JOIN. Grep raw SQL or `where()` calls; check if indexed. -- **`SELECT *`** when only specific columns are serialized. -- **Unbounded queries**: no LIMIT on user-facing list endpoints, `.findAll()`, `.find({})`. -- **Missing pagination** on collection endpoints. -- **Transactions held open** during slow operations (network calls, file I/O inside the transaction). +- A path that launches where it used to reuse: a session rebuilt per request, a new context or page per poll tick, a fallback engine started with too little budget to reach the page. Sessions exist so one solve is reused many times (`CLAUDE.md`). +- A browser, context or page not closed on every exit path. Per-request browsers are closed in the engines' `finally` (`ChromeEngine.solve`, `StealthEngine.solve`); an early return that skips it leaks one browser per request. +- A new per-launch lookup. The stealth launch sends `geo.browser_identity` through `asyncio.to_thread`, and `src/geo.py` caches per proxy; an uncached lookup costs every launch. -## Memory +## The request budget -- Listeners, subscriptions, timers, intervals added without cleanup (`addEventListener` without `removeEventListener`, `setInterval` without `clearInterval`, RxJS `.subscribe()` without `.unsubscribe()`). -- Loading entire files or tables into memory when only a subset is needed. -- Long-lived closures capturing more scope than necessary (class instances captured in event handlers). -- Unbounded caches: `Map` / dict / `HashMap` that only gets `.set()`, no eviction or size limit. -- Streams or file handles not closed. +- `maxTimeout` is one budget for the whole request. `_resolve_challenge` splits what is left evenly across the planned engines and skips a fallback under `_MIN_ENGINE_SECONDS`; `budget.solve_deadline` keeps `SOLVE_MARGIN_SECONDS` back to build the response. +- Flag an engine handed the full budget instead of its share, a wait or retry loop with no deadline (the ledger records an unbounded Chrome Turnstile loop that needed one), a fixed sleep that ignores the remaining budget, or work after the deadline that the margin does not cover. -## Computation +## Threads, the loop, and locks -- Work repeated inside loops that could be hoisted (function calls, regex compilation, object creation in `map`). -- Synchronous blocking on the main thread: `fs.readFileSync`, `execSync`, CPU-heavy work without worker threads. -- Missing early returns when the answer is already known. -- Sorting or filtering large datasets on every render or request instead of caching. +- **The stealth loop is one thread** (`stealth-loop`). Every stealth request, launch and teardown runs on it, so anything blocking in a coroutine (`time.sleep`, a sync lookup, CPU-heavy parsing) stalls every stealth request at once. Requests on one stealth session also serialize on `StealthContext.lock`. +- **Waitress threads are few.** `flaresolverr.py` calls `serve` with no `threads=`, so waitress's default pool of four applies, and each solve holds its thread for the whole solve (Chrome under `func_timeout`, stealth blocked in `AsyncRuntime.run`). Anything that lengthens a solve cuts throughput for every client. +- **SessionStore lock scope** (`src/sessions.py`). Build and teardown run outside `self._lock` by design, per its docstring. A browser round trip or teardown moved under that lock blocks every create, get, destroy and the reaper. A session taken with `get` and not released with `end_use` in a `finally` is pinned for good, since the reaper and the cap both skip `in_use` sessions. +- **The passthrough lock** (`src/passthrough.py`). One module `_lock` guards `_cache` and `_inflight` for a `ThreadingHTTPServer` (a thread per connection). The cache-hit branch of `_Handler._handle` already calls `_send` while holding it, so one slow client stalls every passthrough request; flag any new socket write or solve under that lock. Concurrent requests for one path coalesce on `_Pending`; breaking that turns N identical requests into N solves. -## Network and I/O +## Unbounded growth -- Sequential awaits that could run in parallel. Fix: `Promise.all`, `asyncio.gather`, goroutines. -- Missing request timeouts (`fetch`, `axios`, `http.get` without timeout config). -- No retry-with-backoff for transient failures. -- Over-fetching (sending whole objects when partial data would do). -- Missing compression on responses over 1KB. -- No caching headers on static or rarely-changing responses. +Nothing restarts the process, so a dict keyed by host, path or session id that only grows is a leak. -## Frontend - -- Re-renders: inline object or function props (`onClick={() => ...}`), missing `key`, state updates that don't need to propagate. -- Images without `loading="lazy"`, `srcset`, or size optimization. -- Whole-library imports for one function (`import _ from 'lodash'` instead of `import debounce from 'lodash/debounce'`). -- Layout thrashing: interleaving DOM reads and writes in a loop. -- Animations triggering layout or paint instead of `transform` and `opacity`. -- Render-blocking CSS or JS in the critical path. - -## Concurrency - -- Shared mutable state without synchronization. -- Lock contention: holding locks during I/O or long computations. -- Unbounded worker, goroutine, or thread creation. Use a pool. -- Missing connection pooling for DB or HTTP clients. +- Precedents for the fix: Prometheus labels capped at `_MAX_DOMAIN_LABELS` (`src/bottle_plugins/prometheus_plugin.py`), the passthrough cache capped in bytes (`_cache_store`, `_MAX_BODY_SHARE`), PDFs capped at `_MAX_PDF_BYTES`. +- `_DOMAIN_ENGINE` in `flaresolverr_service.py` is known to be unbounded. Don't re-report it on an unrelated diff; flag a change that copies its shape or makes it grow faster. +- Session count is bounded only by `SESSION_MAX` per engine and `SESSION_TTL_MINUTES`; a change that bypasses `SessionStore` bypasses both. ## What NOT to flag -- Micro-optimizations with no measurable impact. -- Premature optimization in code that runs rarely or handles small data. -- "This could be faster in theory" without evidence it's a real bottleneck. -- Style preferences disguised as performance concerns. +- Micro-optimizations, and Python-level cost next to a browser round trip. +- Code that runs once at startup (`test_browser_installation`) or once per reaper interval, unless egregious. +- The measured waits: `_CHALLENGE_CONFIRM_SECONDS`, `_NETWORKIDLE_MS`, `_CLICK_COOLDOWN_SECONDS`, `_POLL_SECONDS`, `_WIDGET_RENDER_SECONDS`. A wait that looks slow there is what lets the challenge clear (`CLAUDE.md`, "Architecture (non-obvious)"). Ask for a measurement instead. +- Upstream-inherited code outside the diff (`src/undetected_chromedriver/`, the clearing cores). +- "This could be faster in theory" without a frequency-times-cost argument. ## Output format @@ -94,12 +72,12 @@ End with the single highest-impact fix to do first. **Verbose**: For each finding: -- **Impact**: High / Medium / Low, with WHY ("runs per request", "called once at startup, low impact"). +- **Impact**: High / Medium / Low, with WHY ("blocks the stealth loop on every request", "once at startup, low impact"). - **File:Line**: exact location. -- **Issue**: what's slow ("await inside a `for` loop makes N sequential DB calls for N items"). +- **Issue**: what's slow ("`time.sleep` inside `_wait_until_cleared` freezes every other stealth solve for its duration"). - **Fix**: specific code change. - **Confidence**: 0 to 100. End with the single highest-impact fix if they can only do one thing. -Either way, apply the ≥80 confidence filter internally and drop findings below it. +Either way, apply the >=80 confidence filter internally and drop findings below it. diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md index 957c753..caec888 100644 --- a/.claude/agents/security-reviewer.md +++ b/.claude/agents/security-reviewer.md @@ -1,6 +1,6 @@ --- name: security-reviewer -description: Reviews code changes for security vulnerabilities. Use for PR review, pre-deploy verification, or audit of recently changed files. +description: Reviews Solverr changes for untrusted /v1 and passthrough input reaching a browser, secrets or solved cookies leaking into logs, proxy credentials left on disk, and accidental exposure. Use for PR review or audit of recently changed files. tools: - Read - Grep @@ -8,79 +8,59 @@ tools: - Bash --- -You are a senior security engineer reviewing code for vulnerabilities. This is static analysis. Flag patterns that look vulnerable, explain the attack vector, and when in doubt flag with a note. +You are a security engineer reviewing Solverr, a self-hosted bypass proxy: Python 3.14, `bottle` + `waitress` serving the FlareSolverr `/v1` API on 8191, an optional passthrough on 8888, and two real browsers (`chrome`: Selenium + vendored undetected_chromedriver; `stealth`: Camoufox via invisible_playwright on one asyncio loop thread in `src/async_runtime.py`). The threat surface is anyone who can reach those ports steering a browser, and secrets leaking into logs or onto disk. `.claude/rules/security.md` is the project's own baseline; enforce it. + +This is static analysis. Flag patterns that look vulnerable, explain the attack vector, and when in doubt flag with a note. ## Operating principles - State assumptions explicitly. If you can't tell whether input is trusted, say so. - Surgical scope. Review what changed; only flag pre-existing issues if the new code makes them exploitable. -- Verify before flagging. Cite file:line, name the attack vector, give a sample payload when relevant. +- Verify before flagging. Cite file:line and name the attack vector. - Confidence threshold. Only ship findings you're at least 80% sure are exploitable. ## How to review -Run `git diff --name-only`, read each changed file, grep the codebase for related patterns (one SQL injection often means more elsewhere). Cover every category below; skip nothing. - -## Injection - -- **SQL**: string concatenation or interpolation in queries (`"... WHERE id=" + id`, `f"WHERE id={id}"`, template literals). Fix: parameterized queries (`?`, `$1`, named params). -- **Command**: user input reaching shell execution (`exec("ls " + userInput)`, `os.system(f"ping {host}")`). Fix: array-form APIs (`execFile`, `subprocess.run([...])`). -- **XSS**: user input rendered without escaping (`innerHTML = userInput`, `dangerouslySetInnerHTML`, `v-html`, Blade `{!! $var !!}`, `document.write`). Fix: framework text rendering (JSX, Vue `{{ }}`, Go `html/template`). -- **Template**: user input as template content (`render_template_string(user_input)`). Fix: never pass user input as template body. -- **Path traversal**: user input in file paths (`fs.readFile("/uploads/" + filename)` and `../../etc/passwd`). Fix: allowlist + `path.resolve()` + verify prefix, reject `..`. - -## Authentication - -- Password compare with `==` or `===` instead of constant-time (`timingSafeEqual`, `hmac.compare_digest`). -- Session tokens in localStorage (XSS-readable) instead of httpOnly cookies. -- JWTs without `exp` claim. -- Password hashing with MD5, SHA1, SHA256 instead of bcrypt, scrypt, argon2. -- Hardcoded credentials: grep for `password =`, `secret =`, `apiKey =`, `token =` with string literals. -- Missing rate limiting on login, signup, and password reset endpoints. +Run `git diff --name-only`, read each changed file, grep the codebase for related patterns (one unsafe pattern often means more elsewhere). -## Authorization +## No auth, by design -- IDOR: lookups using user-supplied ID without checking ownership (`getOrder(req.params.id)` without `WHERE userId = currentUser`). -- Endpoints serving data without role or permission checks. -- Privilege escalation: user can set their own role in the request body. -- Frontend-only authorization (UI-checked but server doesn't re-verify). +Solverr has no authentication, and exposing it is the deployer's job (`security.md`). Do not propose adding auth. Flag accidental exposure instead: a new listener or endpoint, a default flipped on (`PASSTHROUGH_ENABLED`, `LOG_HTML`), or an endpoint that hands back local state such as files or environment. -## Data exposure +## Untrusted input boundaries -- Secrets in code: `API_KEY`, `SECRET`, `PASSWORD`, `TOKEN` assigned to literals. -- PII in logs: `console.log(user)`, `logger.info(request.body)`. -- Stack traces in responses: `res.json({ error: err.stack })`, unhandled error middleware that leaks internals. -- Verbose errors revealing schema, file paths, or service names. +- **The `/v1` body.** `validate_request_types` (`src/dtos.py`) types every declared field once, in `_controller_v1_handler`. Code that reads a field before that call, or one exempted in `_TYPE_OVERRIDES`, is unvalidated. +- **`url`.** Only `http(s)` may reach a browser: `_validate_url` in `src/flaresolverr_service.py`, called by `_cmd_request_get` and `_cmd_request_post`. Before v1.2.1 a `file://` URL came back in `solution.response`. A new navigating path that skips it, or any loosening of the anchored `_HTTP_URL` regex, is Critical. +- **`postData`.** `build_post_html` (`src/postform.py`) builds an auto-submitting form that both engines load as a `data:text/html` URL. The action is `escape(url, quote=True)` and every field `escape(quote(...))`; dropping either lets a caller inject markup and script into a page the browser runs. +- **`proxy`.** Validated once in `geo.proxy_to_config`, which fails closed on a malformed value (a string proxy used to launch unproxied while reporting success). A new path reading `req.proxy` without it bypasses that. +- **`cookies`.** A list per `validate_request_types`; the entries go to `driver.add_cookie` as sent, or through `_to_playwright_cookies` (a key filter plus domain anchoring). Anything that reads a cookie field into something other than the browser jar is a new boundary. +- **The passthrough** (`src/passthrough.py`). The target host must be in `PASSTHROUGH_ALLOWED_HOSTS` (`_split_host`, `_ALLOWED_HOSTS`); anything else goes to the default mirror, never a caller-chosen host. That allow list is the only thing keeping it from being an open proxy, so a target host taken from anywhere else (a header, the query, a redirect) is Critical. +- **Shell.** Request input never reaches a command string; navigation uses the driver or page API. -## Dependencies +## Secrets and logging -- `npm install` / `pip install` without pinned versions in CI. -- Postinstall scripts executing arbitrary code. -- CDN imports without integrity hashes (SRI). -- Run `npm audit` or `pip audit` if available. +- Never log `PROXY_PASSWORD`, `CAPTCHA_API_KEY`, or returned cookies (`cf_clearance`, `__ddg2_`). Once `flaresolverr.py` or `config.env_proxy` has filled in the proxy, the dict carries the password, so logging `req.proxy`, a `proxy_config`, or a whole request is the same leak. +- Known inherited site: `controller_v1_endpoint` logs the whole request at INFO and the whole response at DEBUG, lines kept byte-identical with FlareSolverr. Don't re-report it on an unrelated diff; flag a change that adds another such line or widens these. +- Exception text reaches both the response and the error log as `"Error: " + str(e)`. An exception built from a credentialed proxy URL or a cookie value leaks it to both. +- `LOG_HTML=true` (`utils.get_config_log_html`) dumps page HTML and stays off by default. +- Hardcoded credentials or API keys. -## Cryptography +## Credentials on disk -- MD5 / SHA1 used for security (not just checksums). -- `Math.random()` or `random.random()` for security tokens. Fix: `crypto.randomBytes`, `secrets.token_hex`. -- Hardcoded keys or IVs. -- ECB mode for block ciphers. -- Missing HTTPS enforcement. +- An authenticated Chrome proxy goes through a generated extension (`utils.create_proxy_extension`) whose temp dir holds the username and password in plaintext. `get_webdriver` removes it in a `finally`, a recorded divergence from upstream, which cleaned up only after a successful launch. A launch path that skips that `finally` leaves credentials in the temp directory. -## Input validation +## Browser and dependencies -- Missing validation on request body fields before use. -- ReDoS: nested quantifiers like `(a+)+`, `(a|b)*c` on user input. -- `parseInt(userInput)` without checking NaN. -- Missing length limits on strings (DoS via large payloads). -- Missing Content-Type validation on file uploads. +- `page.evaluate` or `execute_script` with a string built from request input. +- A launch pref that weakens isolation. Firefox's COOP/COEP stay on by recorded decision (`docs/dev/upstream-sync.md`). +- `invisible-playwright` loosened from its exact pin in `requirements.txt`. It carries the patched Firefox, so a floor lets an unattended bump change the browser. ## What NOT to flag -- Theoretical attacks with no realistic path (timing attacks against admin-only endpoints behind VPN). -- Pre-existing issues outside the diff unless the new code makes them exploitable. +- Missing auth, rate limiting, CSRF or session fixation. No auth is the design. +- A caller steering the browser to any `http(s)` host, private addresses included. `/v1` is a proxy; that is why exposure is the deployer's problem. +- Upstream code outside the diff: `src/undetected_chromedriver/`, `src/tests.py`, `src/tests_sites.py`, `src/bottle_plugins/`, and upstream's Chrome launch flags in `get_webdriver` (`--no-sandbox`, `--ignore-certificate-errors`). Also anything under "Deliberately different" in `docs/dev/upstream-sync.md`. - Defense-in-depth nice-to-haves when the primary defense is sound. -- Style or linter-territory issues. ## Output format @@ -99,10 +79,10 @@ End with a single sentence naming the highest-severity blocker, or "no issues fo For each finding: - **Severity**: Critical / High / Medium / Low. - **File:Line**: exact location. -- **Issue**: attack vector ("an attacker can send `../../../etc/passwd` as filename to read arbitrary files"). +- **Issue**: attack vector ("a `url` of `file:///etc/passwd` passes the new check and the file comes back in `solution.response`"). - **Fix**: specific code change. - **Confidence**: 0 to 100. If no issues, say so explicitly. Don't invent. -Either way, apply the ≥80 confidence filter internally. This tool is not a substitute for a professional audit. +Either way, apply the >=80 confidence filter internally. This tool is not a substitute for a professional audit. diff --git a/.claude/hooks/auto-test.sh b/.claude/hooks/auto-test.sh deleted file mode 100644 index 11ada26..0000000 --- a/.claude/hooks/auto-test.sh +++ /dev/null @@ -1,148 +0,0 @@ -#!/bin/bash -# Finds and runs the matching test file after Claude edits a source file. -# PostToolUse hook for Edit|Write. -# Silent on success. Only emits output when tests fail, so passing tests -# contribute zero tokens. Skips test files themselves, config files, and -# non-testable extensions. - -# Requires jq for JSON parsing. -if ! command -v jq >/dev/null 2>&1; then - exit 0 -fi - -INPUT=$(cat) -FILE_PATH=$(echo "$INPUT" | jq -r '.tool_input.file_path // empty') - -if [ -z "$FILE_PATH" ] || [ ! -f "$FILE_PATH" ]; then - exit 0 -fi - -BASENAME=$(basename "$FILE_PATH") -EXTENSION="${BASENAME##*.}" -NAME="${BASENAME%.*}" -DIR=$(dirname "$FILE_PATH") - -# Skip if the edited file IS a test file. -case "$BASENAME" in - *.test.*|*.spec.*|*_test.*|*_spec.*|test_*|spec_*) exit 0 ;; -esac - -# Skip config, style, and non-code files. -case "$EXTENSION" in - json|yaml|yml|toml|ini|cfg|env|md|txt|css|scss|less|svg|png|jpg|ico|html) exit 0 ;; -esac - -# Skip files in non-testable directories. -case "$FILE_PATH" in - */.claude/*|*/public/*|*/static/*|*/assets/*|*/__mocks__/*) exit 0 ;; -esac - -# Find project root. -find_project_root() { - local dir="$PWD" - while [ "$dir" != "/" ]; do - if [ -f "$dir/package.json" ] || [ -f "$dir/pyproject.toml" ] || [ -f "$dir/Cargo.toml" ] || [ -f "$dir/go.mod" ] || [ -d "$dir/.git" ]; then - echo "$dir" - return - fi - dir=$(dirname "$dir") - done - echo "$PWD" -} - -ROOT=$(find_project_root) -STEM="$NAME" - -# Search for a matching test file in the usual conventions. -find_test_file() { - local stem="$1" - local ext="$2" - - local patterns=( - "${stem}.test.${ext}" - "${stem}.spec.${ext}" - "${stem}_test.${ext}" - "${stem}_spec.${ext}" - "test_${stem}.${ext}" - ) - - # Same directory first. - for pattern in "${patterns[@]}"; do - [ -f "${DIR}/${pattern}" ] && { echo "${DIR}/${pattern}"; return; } - done - - # __tests__ subdirectory (Jest convention). - for pattern in "${patterns[@]}"; do - [ -f "${DIR}/__tests__/${pattern}" ] && { echo "${DIR}/__tests__/${pattern}"; return; } - done - - # Parallel test directory structure (src/foo.ts -> tests/foo.test.ts). - local rel_dir="${DIR#$ROOT/}" - local test_rel_dir - for test_root in "tests" "test" "__tests__" "spec"; do - test_rel_dir=$(echo "$rel_dir" | sed "s|^src/|${test_root}/|;s|^lib/|${test_root}/|") - for pattern in "${patterns[@]}"; do - [ -f "${ROOT}/${test_rel_dir}/${pattern}" ] && { echo "${ROOT}/${test_rel_dir}/${pattern}"; return; } - done - done - - # Broad search as last resort, depth-limited to stay fast. - local found - for pattern in "${patterns[@]}"; do - found=$(find "$ROOT" -maxdepth 5 -name "$pattern" -not -path "*/node_modules/*" -not -path "*/.git/*" -print -quit 2>/dev/null) - [ -n "$found" ] && { echo "$found"; return; } - done -} - -TEST_FILE=$(find_test_file "$STEM" "$EXTENSION") - -if [ -z "$TEST_FILE" ]; then - # No matching test found, not an error. - exit 0 -fi - -# Make path relative for cleaner output if we end up emitting failure logs. -REL_TEST="${TEST_FILE#$ROOT/}" - -# Run tests, capture output, only emit on failure. -# Use default (non-verbose) reporters to keep failure logs tight. -OUTPUT="" -EXIT=0 -case "$EXTENSION" in - js|jsx|ts|tsx|mjs|cjs) - if [ -f "$ROOT/node_modules/.bin/vitest" ]; then - OUTPUT=$(cd "$ROOT" && npx vitest run "$REL_TEST" 2>&1); EXIT=$? - elif [ -f "$ROOT/node_modules/.bin/jest" ]; then - OUTPUT=$(cd "$ROOT" && npx jest "$REL_TEST" 2>&1); EXIT=$? - elif [ -f "$ROOT/node_modules/.bin/mocha" ]; then - OUTPUT=$(cd "$ROOT" && npx mocha "$REL_TEST" 2>&1); EXIT=$? - else - OUTPUT=$(cd "$ROOT" && npm test -- "$REL_TEST" 2>&1); EXIT=$? - fi - ;; - py) - if command -v pytest >/dev/null 2>&1; then - OUTPUT=$(cd "$ROOT" && pytest "$REL_TEST" 2>&1); EXIT=$? - elif command -v python3 >/dev/null 2>&1; then - OUTPUT=$(cd "$ROOT" && python3 -m unittest "$REL_TEST" 2>&1); EXIT=$? - elif command -v python >/dev/null 2>&1; then - OUTPUT=$(cd "$ROOT" && python -m unittest "$REL_TEST" 2>&1); EXIT=$? - fi - ;; - go) - OUTPUT=$(cd "$DIR" && go test ./... 2>&1); EXIT=$? - ;; - rs) - OUTPUT=$(cd "$ROOT" && cargo test 2>&1); EXIT=$? - ;; - *) - exit 0 - ;; -esac - -if [ "$EXIT" -ne 0 ]; then - echo "auto-test: failures in $REL_TEST" - echo "$OUTPUT" -fi - -exit 0 diff --git a/.claude/hooks/block-dangerous-commands.sh b/.claude/hooks/block-dangerous-commands.sh index 6faa178..a0f08a5 100644 --- a/.claude/hooks/block-dangerous-commands.sh +++ b/.claude/hooks/block-dangerous-commands.sh @@ -43,6 +43,9 @@ contains_cmd() { printf '%s' "$COMMAND" | grep -qE "$1"; } contains_icmd() { printf '%s' "$COMMAND" | grep -qiE "$1"; } # ── Git push protections ──────────────────────────────────────────────── +# The session's cwd, which is where the command actually runs. The hook's own cwd is always the +# project dir, so without this a push from a worktree is judged by the main tree's branch. +SESSION_CWD=$(printf '%s' "$INPUT" | jq -r '.cwd // empty' 2>/dev/null || true) if contains_cmd '(^|[;&|()]+[[:space:]]*)git[[:space:]]+push'; then # Explicit refspec to a protected branch (origin main, :main, HEAD:main, remote branch) if contains_cmd "git[[:space:]]+push[[:space:]]+[^[:space:]]+[[:space:]]+([^[:space:]]*:)?($BR_REGEX)(\$|[[:space:]])"; then @@ -55,7 +58,7 @@ if contains_cmd '(^|[;&|()]+[[:space:]]*)git[[:space:]]+push'; then fi # Bare `git push` while on protected branch if contains_cmd 'git[[:space:]]+push[[:space:]]*($|[;&|])'; then - CURRENT=$(git branch --show-current 2>/dev/null || true) + CURRENT=$(git -C "${SESSION_CWD:-.}" branch --show-current 2>/dev/null || git branch --show-current 2>/dev/null || true) if [ -n "$CURRENT" ] && printf '%s' ",$PROTECTED_BRANCHES," | grep -q ",$CURRENT,"; then emit_deny "Blocked: you are on '$CURRENT' (a protected branch). Switch to a feature branch." fi @@ -67,6 +70,16 @@ if contains_cmd '(^|[;&|()]+[[:space:]]*)git[[:space:]]+push'; then fi fi +# ── Merging is never the agent's call ─────────────────────────────────── +# Every PR is merged by a person, with a merge commit. To GitHub a PR merge is a legitimate +# action, so no ruleset can express this; the matcher here is the only guard. +if contains_cmd '(^|[;&|()]+[[:space:]]*)gh[[:space:]]+pr[[:space:]]+merge'; then + emit_deny "Blocked: merging a PR is the owner's call. Open the PR and stop." +fi +if contains_cmd 'gh[[:space:]]+api[^;&|]*pulls/[0-9]+/merge'; then + emit_deny "Blocked: merging a PR through the API is the owner's call. Open the PR and stop." +fi + # ── Destructive filesystem operations ─────────────────────────────────── # rm -rf targeting root, home, $HOME, $VAR (any unresolved expansion), or parent traversal. # We normalise quotes before matching so "my folder", '$HOME/trash', etc. Are all inspected. @@ -79,6 +92,19 @@ if printf '%s' "$CMD_NOQUOTE" | grep -qE 'rm[[:space:]]+(-[a-zA-Z]+[[:space:]]+) emit_deny "Blocked: recursive delete targeting a system directory." fi +# ── Secret files are never read through the shell ─────────────────────── +# The permissions deny list covers the Read, Write and Edit tools only, and auto mode routes file +# reads through the shell instead, so `cat .env` walks straight past it. Only this hook sees the +# command text. The reader must sit in command position (line start or after an operator): +# several reader names are ordinary English words, and matching them mid-sentence rejected +# commit messages. +SECRET_READERS='cat|head|tail|sed|awk|grep|rg|less|more|strings|xxd|od|base64|cp|mv|scp|curl|type|gc|Get-Content|Select-String|Copy-Item' +SECRET_TARGETS='(^|[[:space:]=/\\])\.env([[:space:]./]|$)|\.(pem|key|p12|pfx|jks|keystore)([[:space:]]|$)|(^|[[:space:]=/\\])secrets[/\\]|id_rsa|deny-names\.local' +if printf '%s' "$CMD_NOQUOTE" | grep -qiE "(^|[;&|(])[[:space:]]*($SECRET_READERS)[[:space:]]" \ + && printf '%s' "$CMD_NOQUOTE" | grep -qE "$SECRET_TARGETS"; then + emit_deny "Blocked: that reads a secret file (a .env, a key or certificate, secrets/, or the local deny list). Open it yourself if you need its contents." +fi + # ── PowerShell destructive operations ─────────────────────────────────── # PowerShell has its own spelling for everything above and the POSIX patterns # see none of it. Parameter names may be truncated (-Recurse accepts -Rec), so @@ -152,9 +178,11 @@ fi # Disk / partition. Note: only REDIRECTIONS to /dev/ are destructive. `2>/dev/null` is not. # Pattern matches: `>[ ]*/dev/` but NOT `2>/dev/null` or `&>/dev/null` style for fd-null. -# Strategy: match `>` optionally with whitespace, followed by /dev/, EXCLUDING /dev/null and /dev/stderr/stdout. -if printf '%s' "$COMMAND" | grep -qE '(^|[^0-9&])>[[:space:]]*/dev/[a-zA-Z][a-zA-Z0-9]*' \ - && ! printf '%s' "$COMMAND" | grep -qE '>[[:space:]]*/dev/(null|stdout|stderr|tty|zero|random|urandom)([[:space:]]|$)' ; then +# Strategy: delete the harmless redirects first, then match `>` followed by /dev/ on what is +# left. Excluding them on the whole command failed both ways: `>/dev/null;` read as unsafe because +# the exclusion wanted whitespace after it, and one safe redirect cleared a dangerous one later on. +CMD_SANS_SAFE=$(printf '%s' "$COMMAND" | sed -E 's#>[[:space:]]*/dev/(null|stdout|stderr|tty|zero|random|urandom)##g') +if printf '%s' "$CMD_SANS_SAFE" | grep -qE '(^|[^0-9&])>[[:space:]]*/dev/[a-zA-Z][a-zA-Z0-9]*' ; then emit_deny "Blocked: redirection into a raw device file can destroy data." fi if contains_cmd '(^|[;&|[:space:]])(mkfs|mkfs\.[a-z0-9]+)([[:space:]]|$)' \ diff --git a/.claude/hooks/context-recovery.sh b/.claude/hooks/context-recovery.sh deleted file mode 100644 index bfe9392..0000000 --- a/.claude/hooks/context-recovery.sh +++ /dev/null @@ -1,111 +0,0 @@ -#!/bin/bash -# Re-injects critical project rules after context compaction. -# Used as a SessionStart hook with matcher "compact". -# -# When Claude's context window fills up, compaction summarizes the conversation -# and loses specific details. This hook restores your non-negotiable project -# rules so Claude stays aligned even after compaction. -# -# Customize the RULES section below with your project-specific requirements. - -# ────────────────────────────────────────────── -# Find project root -# ────────────────────────────────────────────── - -find_project_root() { - local dir="$PWD" - while [ "$dir" != "/" ]; do - if [ -f "$dir/package.json" ] || [ -f "$dir/pyproject.toml" ] || [ -f "$dir/Cargo.toml" ] || [ -f "$dir/go.mod" ] || [ -d "$dir/.git" ]; then - echo "$dir" - return - fi - dir=$(dirname "$dir") - done - echo "$PWD" -} - -ROOT=$(find_project_root) - -# ────────────────────────────────────────────── -# Dynamic context (same as session-start.sh) -# ────────────────────────────────────────────── - -CONTEXT="" - -BRANCH=$(git branch --show-current 2>/dev/null) -if [ -n "$BRANCH" ]; then - CONTEXT="Branch: $BRANCH" -fi - -LAST_COMMIT=$(git log --oneline -1 2>/dev/null) -if [ -n "$LAST_COMMIT" ]; then - CONTEXT="$CONTEXT | Last commit: $LAST_COMMIT" -fi - -CHANGES=$(git status --porcelain 2>/dev/null | wc -l | tr -d ' ') -if [ "$CHANGES" -gt 0 ] 2>/dev/null; then - CONTEXT="$CONTEXT | Uncommitted changes: $CHANGES files" -fi - -# ────────────────────────────────────────────── -# Re-inject critical project rules -# ────────────────────────────────────────────── - -cat <<'RULES' -=== CONTEXT RECOVERED AFTER COMPACTION === - -CRITICAL PROJECT RULES (restored automatically. Do not ignore): - -1. TESTING - - Run the specific test file after changes, not the full suite. - - Tests must verify behavior, not implementation details. - - Prefer real implementations over mocks. Only mock at system boundaries. - - One assertion per test. Arrange-Act-Assert structure. - -2. CODE QUALITY - - Don't add features beyond what was asked. - - No dead code or commented-out blocks. - - Functions do one thing. No magic values. - - Named exports over default exports. - -3. WORKFLOW - - Run typecheck after making code changes. - - Prefer fixing root causes over workarounds. - - Don't modify generated files (*.gen.ts, *.generated.*). - - Don't modify lock files, .env files, or hook scripts. - -4. SECURITY - - Never commit secrets, tokens, or credentials. - - Validate all user input at system boundaries. - - Parameterized queries only. No string interpolation in SQL. - -5. GIT - - Don't push directly to main/master. - - No force pushes (use --force-with-lease if needed). - - Create feature branches for all work. - -RULES - -# ────────────────────────────────────────────── -# Append dynamic context -# ────────────────────────────────────────────── - -if [ -n "$CONTEXT" ]; then - echo "" - echo "Current state: $CONTEXT" -fi - -# ────────────────────────────────────────────── -# Re-read CLAUDE.md if it exists (belt and suspenders) -# ────────────────────────────────────────────── - -if [ -f "$ROOT/CLAUDE.md" ]; then - echo "" - echo "=== CLAUDE.md (re-injected) ===" - cat "$ROOT/CLAUDE.md" -fi - -echo "" -echo "=== END CONTEXT RECOVERY ===" - -exit 0 \ No newline at end of file diff --git a/.claude/hooks/protect-files.sh b/.claude/hooks/protect-files.sh index e4b0c2d..25a8048 100644 --- a/.claude/hooks/protect-files.sh +++ b/.claude/hooks/protect-files.sh @@ -20,6 +20,8 @@ fi INPUT=$(cat) FILE_PATH=$(printf '%s' "$INPUT" | jq -r '.tool_input.file_path // empty' 2>/dev/null || true) [ -z "$FILE_PATH" ] && exit 0 +# On Windows the tools pass backslash paths, which no directory pattern below would match. +FILE_PATH=${FILE_PATH//\\//} BASENAME=$(basename -- "$FILE_PATH") # Case-insensitive comparison copy diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/22-allow-ps-remove-item-subdir.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/22-allow-ps-remove-item-subdir.json index 52d194c..adfa3aa 100644 --- a/.claude/hooks/tests/fixtures/block-dangerous-commands/22-allow-ps-remove-item-subdir.json +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/22-allow-ps-remove-item-subdir.json @@ -1,5 +1,5 @@ { "name": "allow PowerShell recursive delete of a concrete subdirectory", - "stdin": { "tool_input": { "command": "Remove-Item -Recurse -Force .worktrees/loop-12" } }, + "stdin": { "tool_input": { "command": "Remove-Item -Recurse -Force .worktrees/scratch" } }, "expect_exit": 0 } diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/29-allow-docker-rm-force.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/29-allow-docker-rm-force.json index 287e2d2..1501c2c 100644 --- a/.claude/hooks/tests/fixtures/block-dangerous-commands/29-allow-docker-rm-force.json +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/29-allow-docker-rm-force.json @@ -1,5 +1,5 @@ { - "name": "allow docker rm -f of a loop container", - "stdin": { "tool_input": { "command": "docker rm -f solverr-loop12 && docker rmi solverr:loop12" } }, + "name": "allow docker rm -f of a test container", + "stdin": { "tool_input": { "command": "docker rm -f solverr-livetest && docker rmi solverr:livetest" } }, "expect_exit": 0 } diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/30-allow-git-worktree-remove-force.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/30-allow-git-worktree-remove-force.json index 7c90ee6..056a9ca 100644 --- a/.claude/hooks/tests/fixtures/block-dangerous-commands/30-allow-git-worktree-remove-force.json +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/30-allow-git-worktree-remove-force.json @@ -1,5 +1,5 @@ { "name": "allow git worktree remove --force", - "stdin": { "tool_input": { "command": "git worktree remove .worktrees/loop-12 --force" } }, + "stdin": { "tool_input": { "command": "git worktree remove .worktrees/scratch --force" } }, "expect_exit": 0 } diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json new file mode 100644 index 0000000..3d545e1 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/31-block-gh-pr-merge.json @@ -0,0 +1,6 @@ +{ + "name": "block gh pr merge", + "stdin": { "tool_input": { "command": "gh pr merge 11 -R owner/repo --merge" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "merg"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json new file mode 100644 index 0000000..e450a19 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/32-block-gh-api-merge.json @@ -0,0 +1,6 @@ +{ + "name": "block a PR merge through gh api", + "stdin": { "tool_input": { "command": "gh api -X PUT repos/owner/repo/pulls/11/merge" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "merg"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json new file mode 100644 index 0000000..3fdcb08 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/33-allow-gh-pr-view.json @@ -0,0 +1,5 @@ +{ + "name": "allow reading a PR", + "stdin": { "tool_input": { "command": "gh pr view 11 -R owner/repo --json state,mergeable" } }, + "expect_exit": 0 +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json new file mode 100644 index 0000000..0fe1fb7 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/34-block-cat-env.json @@ -0,0 +1,6 @@ +{ + "name": "block reading a .env through the shell", + "stdin": { "tool_input": { "command": "cat .env" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "secret"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json new file mode 100644 index 0000000..4280939 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/35-block-ps-read-deny-names.json @@ -0,0 +1,6 @@ +{ + "name": "block reading the local deny list through PowerShell", + "stdin": { "tool_name": "PowerShell", "tool_input": { "command": "Get-Content .githooks/deny-names.local" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "secret"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json new file mode 100644 index 0000000..54f8048 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/36-allow-reader-words-in-prose.json @@ -0,0 +1,5 @@ +{ + "name": "allow a commit message that uses reader verbs as words", + "stdin": { "tool_input": { "command": "git commit -m \"docs: say how head and tail of the .env example are read\"" } }, + "expect_exit": 0 +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json new file mode 100644 index 0000000..c2a9682 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/37-allow-devnull-then-semicolon.json @@ -0,0 +1,5 @@ +{ + "name": "allow a /dev/null redirect followed by another command", + "stdin": { "tool_input": { "command": "ls >/dev/null; echo done" } }, + "expect_exit": 0 +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json new file mode 100644 index 0000000..0a547eb --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/38-block-raw-device-after-safe-redirect.json @@ -0,0 +1,6 @@ +{ + "name": "block a raw device write even after a safe redirect", + "stdin": { "tool_input": { "command": "ls >/dev/null && cat x > /dev/sda" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "device"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/39-block-ps-gc-alias-secret.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/39-block-ps-gc-alias-secret.json new file mode 100644 index 0000000..e78de59 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/39-block-ps-gc-alias-secret.json @@ -0,0 +1,6 @@ +{ + "name": "block reading a .env with the PowerShell gc alias", + "stdin": { "tool_name": "PowerShell", "tool_input": { "command": "gc .env" } }, + "expect_exit": 2, + "expect_stdout_contains": ["deny", "secret"] +} diff --git a/.claude/hooks/tests/fixtures/block-dangerous-commands/40-block-type-secrets-backslash.json b/.claude/hooks/tests/fixtures/block-dangerous-commands/40-block-type-secrets-backslash.json new file mode 100644 index 0000000..af244b4 --- /dev/null +++ b/.claude/hooks/tests/fixtures/block-dangerous-commands/40-block-type-secrets-backslash.json @@ -0,0 +1,14 @@ +{ + "name": "block reading a file under secrets/ with type and a backslash path", + "stdin": { + "tool_name": "PowerShell", + "tool_input": { + "command": "type secrets\\token.txt" + } + }, + "expect_exit": 2, + "expect_stdout_contains": [ + "deny", + "secret" + ] +} diff --git a/.claude/hooks/tests/fixtures/protect-files/17-block-claude-hook-edit-backslash.json b/.claude/hooks/tests/fixtures/protect-files/17-block-claude-hook-edit-backslash.json new file mode 100644 index 0000000..6202a47 --- /dev/null +++ b/.claude/hooks/tests/fixtures/protect-files/17-block-claude-hook-edit-backslash.json @@ -0,0 +1,9 @@ +{ + "name": "block editing a hook script given a Windows backslash path", + "stdin": { + "tool_input": { + "file_path": "E:\\repo\\.claude\\hooks\\foo.sh" + } + }, + "expect_exit": 2 +} diff --git a/.claude/hooks/tests/fixtures/protect-files/18-block-secrets-dir-backslash.json b/.claude/hooks/tests/fixtures/protect-files/18-block-secrets-dir-backslash.json new file mode 100644 index 0000000..2b89f67 --- /dev/null +++ b/.claude/hooks/tests/fixtures/protect-files/18-block-secrets-dir-backslash.json @@ -0,0 +1,9 @@ +{ + "name": "block editing inside secrets/ given a Windows backslash path", + "stdin": { + "tool_input": { + "file_path": "E:\\repo\\secrets\\token.txt" + } + }, + "expect_exit": 2 +} diff --git a/.claude/hooks/tests/fixtures/protect-files/19-ask-settings-backslash.json b/.claude/hooks/tests/fixtures/protect-files/19-ask-settings-backslash.json new file mode 100644 index 0000000..cb03cf9 --- /dev/null +++ b/.claude/hooks/tests/fixtures/protect-files/19-ask-settings-backslash.json @@ -0,0 +1,12 @@ +{ + "name": "ask before editing settings.json given a Windows backslash path", + "stdin": { + "tool_input": { + "file_path": "E:\\repo\\.claude\\settings.json" + } + }, + "expect_exit": 2, + "expect_stdout_contains": [ + "ask" + ] +} diff --git a/.claude/rules/architecture.md b/.claude/rules/architecture.md new file mode 100644 index 0000000..caa60e0 --- /dev/null +++ b/.claude/rules/architecture.md @@ -0,0 +1,19 @@ +--- +paths: + - "src/**" +--- + +# Architecture (non-obvious) + +How the pieces fit, and the constraints that look wrong until you know what they were measured against. `CLAUDE.md` carries the one-paragraph version; [engine-layer.md](engine-layer.md) is the law for what is shared between the engines and what is not. + +- Two engines behind one interface (`engines/base.py`): `chrome` (Selenium + vendored undetected_chromedriver, the default) and `stealth` (Camoufox via invisible_playwright + playwright-captcha). The controller auto-falls-back between them and remembers per-host which one cleared it. +- The stealth engine is async Playwright running on ONE background event-loop thread (`async_runtime.py`); persistent Camoufox contexts (sessions) live there so their cookies survive across requests. The server itself is synchronous. +- Sessions: each engine keeps its own pool, both using one `SessionStore` (`sessions.py`) so the lifecycle rules exist once; a background reaper (`session_reaper.py`) closes idle browsers. A session handed out is marked in use under the same lock that found it, which is what keeps the reaper and the cap off a live browser. Solve once, reuse the cookie many times. +- Escalation ladder for an `auto` request: Chrome → Camoufox click-solve → (optional, dormant) paid CAPTCHA API. +- **A Turnstile checkbox is clicked by coordinate, with no JS evaluation.** The widget's iframe sits in a closed shadow root, so `query_selector` cannot find it, but `page.frames` lists it anyway; `frame_element().bounding_box()` gives its rect and `page.mouse` clicks the checkbox. This exists because playwright-captcha's shadow-root traversal uses `evaluate_handle`, and the iframe's CSP blocks eval under Firefox, which silently broke widget solving. Cloudflare's own interstitial builds the widget itself and its frame reports an empty URL, so when no frame matches, the rect comes from the nearest ancestor `div` of the token input instead, which is in the light DOM. Do not reach for `page.evaluate` to measure any of this: running page scripts against a live challenge makes Cloudflare reissue it. +- **A challenge is only over once a clear reading survives a second look.** Cloudflare drops the challenge markup while it issues the next round, so believing the first clear reading returns an intermediate challenge page. +- **`maxTimeout` is one budget for the whole request, split evenly across the planned engines.** It used to be handed to each engine in full, so a fallback could take twice as long as asked and trip the caller's own timeout. An even share is what makes the fallback reachable: giving the first engine everything let it spend the lot, and a request that used to succeed in 133s failed at 120s with the second engine skipped. A quick first engine costs the fallback nothing, since the fallback inherits everything unspent. +- **The POST form is carried to the browser as a `data:text/html,` URL, so its fields are percent-encoded and must stay that way** (`postform.py`). The browser URL-decodes the document before the HTML parser sees it, so a value holding a bare `%` or `#` is otherwise re-read as an escape or truncates the document at the fragment. The `quote()` calls look like double-encoding and are not: removing them breaks POST for those values, measured against a live echo service. +- **Solverr resolves the browser's timezone and language itself (`geo.py`), and hands both engines the same pair.** Left alone, the stealth stack resolves both from the exit IP on every launch, inside the library, uncached, and raises behind a proxy when the lookup fails, which kills the launch; Chrome derived neither, so the two engines disagreed about the country. Passing concrete values returns before that fatal branch. They travel together because the pairing is what a site checks. Chrome follows via `Emulation.setTimezoneOverride` (which moves its ICU clock rather than patching `Intl` in the page) and `--accept-lang`. A failed lookup falls back to `TZ` and `en-US`: a wrong zone still solves, no browser does not. +- **playwright-captcha only ever touches a throwaway page**, and only the paid escalation reaches it now. Preparing a solver injects init scripts (one rewrites `Element.prototype.attachShadow`) that a Cloudflare interstitial will not clear while they are present, and Playwright cannot remove an init script. Verified live: an interstitial clears in ~3s without them and never in 40s with them. diff --git a/.claude/rules/code-quality.md b/.claude/rules/code-quality.md index d866500..31d05eb 100644 --- a/.claude/rules/code-quality.md +++ b/.claude/rules/code-quality.md @@ -9,19 +9,19 @@ alwaysApply: true - **DRY**: before adding a helper, search for an existing equivalent (`postform.py`, `detection.py`, `config.py`). - **YAGNI**: add only what the task needs. No speculative parameters or abstractions for hypothetical callers. - **KISS**: simplest correct solution. Justify complexity with a concrete requirement, not elegance. -- **Fix the defect, not the instance that reproduced.** A bug present in five places is one bug with five sites. Fix all five, or name the ones you left and why. Search for the sibling instances before calling a fix done. The shared spine (`assembly.py`, `pipeline.py`, `budget.py`, `sessions.py`) now holds the rules that used to be written once per engine, so a defect in one of those is a defect for both; what remains genuinely per-engine is each clearing core. +- **Fix the defect, not the instance that reproduced.** A bug present in five places is one bug with five sites. Fix all five, or name the ones you left and why, with one exception: an engine pair is never split. A change a client can observe lands for both engines in the same commit, and the only exit is the named mechanism in [engine-layer.md](engine-layer.md). Search for the sibling instances before calling a fix done. The shared spine (`assembly.py`, `pipeline.py`, `budget.py`, `sessions.py`) now holds the rules that used to be written once per engine, so a defect in one of those is a defect for both; what remains genuinely per-engine is each clearing core. - **Minimal blast radius is measured against the defect, not against the diff.** Leave genuinely unrelated code alone. A small diff is not the goal: when the correct fix needs a helper extracted, a signature changed, or a call site moved, do that instead of threading a workaround through the shape that is already there. -- **Refactor when the fix needs it**, in the same change, with the reason in the commit body. Still no standalone refactor sprints, and still nothing adjacent riding along uninvited. -- **One standing exemption to that ban** (owner, 2026-08-25): the engine layer program in [engine-layer.md](engine-layer.md), whose steps are refactors with no fix attached. It exists because the per-engine duplication produced the same defect in both engines at once, which a fix-shaped change cannot prevent recurring. The exemption covers only the sequenced steps recorded in [engine-layer-architecture.md](../../docs/dev/engine-layer-architecture.md); anything else is still an ordinary refactor and still needs a fix to ride with. +- **Refactor when the fix needs it**, in the same change, with the reason in the commit body. Still no standalone refactor sprints, and still nothing adjacent riding along uninvited. One exception comes from the engine-layer law: a parity gap you notice on an engine surface you are touching is levelled up in that change, unless the owner gates it. That never licenses cleanup on a file just because it was open. +- **One standing exemption to that ban** (owner, 2026-08-25): the engine layer program in [engine-layer.md](engine-layer.md), whose steps are refactors with no fix attached. It exists because the per-engine duplication produced the same defect in both engines at once, which a fix-shaped change cannot prevent recurring. The exemption covers only the sequenced steps recorded in [engine-layer-architecture.md](../../docs/dev/engine-layer-architecture.md); anything else is still an ordinary refactor and still needs a fix to ride with. All six steps are done, so the exemption covers nothing further: a new behaviour-free move needs the owner's approval like any other refactor. - **Prefer the proper fix over the patch.** If the patch is genuinely the right call (a risky area, a release in flight), say so explicitly and record what the proper fix would be. An unstated tradeoff reads as an oversight to whoever finds it next. ## Anti-defaults (counter common Claude tendencies) -- No premature abstractions. Three similar lines beat a helper used once. -- Don't add features beyond what was asked. Refactoring is the different case: do it when the correct fix requires it, not as a separate pass and not as adjacent cleanup. +- No premature abstractions. Three similar lines beat a helper used once. A rule that must hold for both engines is never "used once": it has two callers by definition, so it belongs in the spine. +- Don't add features beyond what was asked. The second engine is not beyond what was asked (write-once, above). Refactoring is the different case: do it when the correct fix requires it, not as a separate pass and not as adjacent cleanup. - Don't stop at the first site that made the bug visible. "The reported case now passes" is not the same as "the bug is fixed". - No dead code or commented-out blocks. Git has history. -- WHY comments, never WHAT. If code needs a "what" comment, rename instead. Docstrings at module/engine boundaries, not every internal function. +- Comments say WHY: why this approach, why not the obvious one, what breaks otherwise. A WHAT comment is allowed when the what is not visible in the code at hand: an invariant, how an upstream or library dependency behaves, what a magic value means, how this piece couples to a distant one. A comment that restates the adjacent code is dead weight; rename instead. Docstrings at module/engine boundaries, not every internal function. - No em dashes in code, comments, or docs. Use commas, parentheses, periods, or colons. - No AI watermarks: no "Co-Authored-By: Claude", no "Generated with Claude Code", no robot-emoji footers. diff --git a/.claude/rules/engine-layer.md b/.claude/rules/engine-layer.md index 0cffadf..360817d 100644 --- a/.claude/rules/engine-layer.md +++ b/.claude/rules/engine-layer.md @@ -24,9 +24,10 @@ Solverr has two upstreams and owes both a mergeable diff. The ledger - **The stealth clearing core is Byparr's.** It shares Byparr's algorithm and its widget constants by name and role (`ANCESTOR_DEPTHS`, `MIN_WIDTH`, `MIN_HEIGHT`, `MAX_HEIGHT`, `COOLDOWN`), and the ledger's Taken section records four separate ports into it. -- **Everything wrapped around those cores is ours, and it is written twice.** Request-option - handling, session lifecycle and result assembly are each implemented once per engine. That is - where the duplication lives, and it is ordinary duplication with no exemption. +- **Everything wrapped around those cores is ours.** Request-option handling, session lifecycle and + result assembly used to be implemented once per engine, which is where the same defect kept + landing twice. They now live once in the spine (the seam-depth table below); what stays per + engine is each adapter, and duplication there is ordinary duplication with no exemption. **The line runs inside each engine, not between them.** The two cores are two mechanisms, not two implementations of one rule, so collapsing them would fork both engines from their upstream and buy @@ -57,13 +58,22 @@ nothing. recorded in the ledger. "The engines are structured differently", "the other side needs a rewrite first" and "no caller needs it yet" are not exits, they are the work. If the second half cannot ship in the same commit, the change goes back to planning as one item covering both. +- **What a clearing core owns alone falls under the clearing-core decline, not write-once.** A + tuning knob that only parameterizes one core's own loop (`BROWSER_WAIT_TIMEOUT`, Chrome's + per-attempt wait) has nothing to tune on the other engine, and a dependency only one engine uses + (Playwright and invisible-playwright for stealth, Selenium for Chrome) moves with a live check + rather than a second-engine half. Both are documented as engine-specific and recorded in the + ledger, never silent. The moment either changes what a client can observe, write-once applies. - **Sharing the implementation is a means, not the rule.** Declining a code collapse stays allowed on cited mechanism grounds (the two clearing cores are the standing example), and it never licenses a behaviour fork. Two implementations that must behave identically are pinned by one conformance test. - **Divergent bits are typed capability slots.** Never a nullable field, never a boolean-flag combination, never a per-engine branch inside shared code. A capability an engine cannot support - is routed to one that can, or refused by name. Never a silent no-op: `tabs_till_verify` quietly + is routed to one that can, or refused by name. One boolean the spine takes from each adapter is + not a combination and is allowed while it is the only divergent bit on its surface + (`turnstile_is_a_challenge` on `pipeline.verdict`); a second one on the same surface turns both + into one typed capability. Never a silent no-op: `tabs_till_verify` quietly doing nothing on the stealth engine is the defect this rule exists to stop. - **A shared component either derives a piece of state or does not own it.** Sharing the storage while each engine interprets it its own way is a fork wearing shared-code clothing, and nobody @@ -74,7 +84,9 @@ nothing. conformance rung is `src/test_engine_conformance.py`, driven by `src/engine_fakes.py`; add to it rather than writing a second per-engine test, and delete the per-engine test it supersedes. - **Parity is the default; a gap needs a ruling to stay open.** A gap you notice on a surface you are - touching is levelled up in that change unless the owner gates it. + touching is levelled up in that change unless the owner gates it. A gate is the owner's ruling + and is never self-issued by whoever is doing the work. A gap that predates this rule is paid + when its surface is next touched, never in a sweep of its own. - **A decline expires with its evidence.** Record the premise with the decline and treat the decline as void once that premise changes. - **Verify by mutation.** A new test is not done until the production clause it names has been diff --git a/.claude/rules/plan-output.md b/.claude/rules/plan-output.md index abc0be3..93264c1 100644 --- a/.claude/rules/plan-output.md +++ b/.claude/rules/plan-output.md @@ -4,7 +4,7 @@ alwaysApply: true # Plan and findings output format -How a research report or implementation plan is written, whether it comes from `/scout`, `/upstream-audit`, or a plan given directly in conversation. The goal is density: keep every technical claim and every `file:line`, cut the words around them. +How a research report or implementation plan is written, whether it comes from `/scout`, `/code-research`, `/upstream-audit`, or a plan given directly in conversation. The goal is density: keep every technical claim and every `file:line`, cut the words around them. This file governs the structure. [prose-style.md](prose-style.md) governs the sentences inside it. @@ -20,7 +20,7 @@ The single binding constraint in two or three sentences: what actually drives th Grouped **High / Medium / Low**. Each finding is a **bolded one-line claim**, then the shortest prose that carries the evidence, with inline `file:line` references. -Prose, not bullet fragments, and a few tight sentences rather than a paragraph. The claim line states the conclusion; the prose exists only to make it checkable. Mark a finding **verified** when re-read directly, **reported** when it came from a subagent and was not re-read. +Detail is tiered by grade. **High** findings get the evidence prose: a few tight sentences, not a paragraph; the claim line states the conclusion and the prose exists only to make it checkable. **Medium and Low** findings are one line each: the claim plus its citation, no supporting prose. If a Medium finding cannot be stated in one line with a citation, it is either High or it is two findings. Mark a finding **verified** when re-read directly, **reported** when it came from a subagent and was not re-read. ### 3. Stale docs @@ -34,14 +34,14 @@ Omit this section for a pure audit with no implementation to propose. ### 5. Open questions -Last section, numbered, each marked **blocking** or **non-blocking**. Give the concrete options, a recommendation, and the reasoning behind it. These get answered before implementation starts, so a question with no options attached is not finished. +Last section, numbered, each marked **blocking** or **non-blocking**. At most three sentences per question: the question, the options as a short phrase each, the recommendation with a one-clause reason. The full tradeoff discussion happens in conversation only if the owner asks for it. A question with no options attached is not finished. ## Rules - **Density over length.** Every sentence carries a fact the reader does not already have. Cut restatement, throat-clearing, and transitions that only announce what is coming. - **Never drop a `file:line` to save space.** References are the payload, prose is the wrapper. Trim the wrapper. -- **No progress narration in the artifact.** "Now let me check", "Terrain mapped", "Six good returns" are working-log material and never appear in the report. +- **No progress narration, in the artifact or around it.** "Now let me check", "Terrain mapped", "Six good returns" are working-log material. In the report they never appear; in the conversation, one sentence when the fan-out starts, then silence until the report. - **Bullets enumerate options; prose carries findings.** Do not fragment a finding into bullets to look shorter. - **Cite it or drop it.** A claim without a `file:line` from code actually read belongs in Open questions, not Findings. - **No em dashes.** Commas, parentheses, periods, colons. -- Cap the artifact around 1500 words. Longer means the question needed splitting, not that the report needed more room. +- Cap the artifact around 700 words, and treat that as a ceiling, not a target. Longer means the question needed splitting, not that the report needed more room. diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 5ebf81b..73f967e 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -5,9 +5,11 @@ alwaysApply: true # Testing - Verify behavior, not implementation. Don't assert mock call counts when output values would do. -- Run the specific test file after changes, not the full suite. Faster feedback, fewer tokens. +- **A rule that must hold for both engines is pinned once, not twice.** Prefer a test over the shared spine both engines call; where the engines are genuinely separate, write one case in `src/test_engine_conformance.py`, which drives each engine through `src/engine_fakes.py`, instead of a hand-maintained pair. A pair drifts. Full rule: [engine-layer.md](engine-layer.md). +- Run the specific test module after a change (`PYTHONPATH=src uv run --no-project python -m unittest test_request_validation`), then the whole browser-free suite before calling it done: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src`. Never a hand-picked list of modules as the final check: it skips the conformance suite. - Flaky test? Fix it or delete it. Never retry to make it pass. -- Prefer real implementations. Mock only at system boundaries (network, filesystem, clock, randomness). -- One assertion per test. Test names describe behavior. Arrange-Act-Assert. No `if` or loops in tests. +- Prefer real implementations. Mock only at system boundaries (network, filesystem, clock, randomness). Patch a polling sleep with a plain function, never a `MagicMock`: the mock records every call and a polling wait turns into unbounded memory that looks like a hang. +- One assertion per test. Test names describe behavior. Arrange-Act-Assert. No `if` or loops in tests; parameterize instead. Here that means `self.subTest` over a fixed tuple of cases or engines, the shape the conformance suite uses, and the one sanctioned loop. - Never assert only that a mock was called without verifying arguments. -- This project uses `unittest` + `webtest` (`src/tests.py`); the full suite launches a real browser and hits live sites, so it's slow and network-dependent. For fast feedback on non-solving changes, prefer `uv run --no-project python -m py_compile ...` and small targeted `unittest` runs over the whole suite. +- **A new test is not done until it has failed.** Delete the production clause it names, see it red, restore the clause (engine-layer.md, "Verify by mutation"). +- `src/tests.py` is upstream's suite: it launches a real browser and hits live sites, so it is slow and network-dependent, and it cannot run in CI. The `src/test_*.py` modules are the browser-free suite CI runs on every pull request. Neither can tell you whether a page still clears a real challenge; that is `/live-check`. diff --git a/.claude/rules/workflow.md b/.claude/rules/workflow.md index 59288bd..fc661f1 100644 --- a/.claude/rules/workflow.md +++ b/.claude/rules/workflow.md @@ -21,9 +21,9 @@ After a code change with any user-facing effect, add a bullet under `## [Unrelea ## Cutting a release (user-initiated) -1. Rename `## [Unreleased]` to `## []`. +1. Rename `## [Unreleased]` to `## []`, and collapse any entries in it that state the same fact: a fix to something added in the same release folds into that addition's entry. 2. Add a fresh empty `## [Unreleased]` above it. -3. Bump `version` in `package.json` to ``, and commit. +3. Bump `version` in `package.json` and the version in the README's response example to ``, and commit. 4. Tag and push: `git tag v && git push origin v`. The tag triggers `release-docker.yml` (builds + pushes the ghcr image) and `release.yml` (creates the GitHub Release from the `[]` section). `release.yml` can also be run manually from the Actions tab (workflow_dispatch) with the version and an optional note. Don't bump the version mid-cycle; only at release-cut. @@ -34,9 +34,11 @@ Create a commit after a change (do not push unless asked). - Subject `type(scope): summary`: a real conventional type (`feat`, `fix`, `docs`, `chore`, `refactor`, `test`, `perf`), imperative, lower-case, no trailing period, `<=72` chars. Scope optional (`chrome`, `stealth`, `sessions`, `docker`). - Non-trivial commits get a body: lead with 1-2 plain-language sentences (what changed and why it matters), then benefit-first bullets. A trivial commit is just the subject. -- No em dashes. No AI watermarks (no `Co-Authored-By: Claude`, no generated-by footer, no robot emoji). +- No em dashes. No AI watermarks (no `Co-Authored-By: Claude`, no generated-by footer, no robot emoji). A `Co-authored-by` trailer for a person is credit and passes the hook; one naming an AI tool is a watermark and is rejected. - **Never a bare `#N`** in the subject or body: it silently links to an issue in this repo. Use the explicit `owner/repo#N` form (`FlareSolverr/FlareSolverr#1626`, `ThePhaseless/Byparr#377`). +**Merging a pull request.** A person merges every pull request, with a merge commit, never squash or rebase. A squash subject ends in ` (#N)`, which the bare-`#N` check rejects on `main`, and a merge commit keeps an outside contributor's commits under their name. The standard for contributors is written out in `CONTRIBUTING.md` and the pull request template; keep both in step with this file. When a contributor's commit message breaks the standard, reword it with `git commit --amend` (which keeps them as the author), push it to their branch with `--force-with-lease`, then merge. Never recommit their change under your own name. + ### Pre-commit checklist Run these against the message before committing. The first four are also enforced by `.githooks/commit-msg`; the rest are on you. @@ -50,7 +52,7 @@ Run these against the message before committing. The first four are also enforce ## Public-facing naming -**Keep the names of the sites Solverr is pointed at out of every public surface**: commit messages, branch names, `README.md`, `CLAUDE.md`, `CHANGELOG.md`, release notes, and the repo description and topics. Solverr is a general-purpose bypass proxy; naming targets makes it read as tooling for one specific site. +**Keep the names of the sites Solverr is pointed at out of every public surface**: commit messages, branch names, `README.md`, `CLAUDE.md`, `CHANGELOG.md`, `CONTRIBUTING.md`, the pull request template, release notes, and the repo description and topics. Solverr is a general-purpose bypass proxy; naming targets makes it read as tooling for one specific site. Use generic wording instead: "a Cloudflare-gated site", "an indexer", "the default mirror", "example-site.tld" in docs and examples. Site names are fine in local test scratch files, in chat, and in a private indexer definition that lives outside this repo. @@ -64,10 +66,11 @@ Inherited exception: `src/tests_sites.py` and `src/tests.py` carry a site list f git config core.hooksPath .githooks ``` -- `commit-msg` enforces the message standard above. -- `pre-commit` lints staged `CHANGELOG.md` and `README.md` for the naming rule, em dashes, and the benefit-first headline format. +- `commit-msg` rejects: a subject that is not `type(scope): summary`; a subject over 72 characters; an em dash anywhere; an AI watermark (an AI `Co-authored-by` trailer, "Generated with", the robot emoji); a bare `#N`; a domain-shaped site name or scraping vocabulary. Merge, revert, fixup and squash commits pass untouched. +- `pre-commit` lints the lines a commit adds to `CHANGELOG.md`, `README.md`, `CONTRIBUTING.md` and `CLAUDE.md` for the naming rule and em dashes, and every `[Unreleased]` entry outside `Other` for a bold headline ending in `.`, `!` or `?`. +- `.githooks/tests/run.sh` proves each rule above still rejects a real violation and passes a clean case. Run it after touching either hook. -Never bypass with `--no-verify`. If a hook fires on something legitimate, fix the hook in the same change. +CI runs all of it: the Standards workflow runs the hook self-test, then `commit-msg` on every non-merge commit and `pre-commit` over the pushed range, and the Tests workflow runs the browser-free suite on every pull request. Never bypass with `--no-verify`. If a hook fires on something legitimate, fix the hook and its self-test in the same change. ## Approach diff --git a/.claude/settings.json b/.claude/settings.json index 073768e..283cb27 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -35,18 +35,27 @@ "Bash(git fetch *)", "Bash(git checkout *)", "Bash(git switch *)", - "Bash(gh pr *)", - "Bash(gh issue *)", + "Bash(gh pr view *)", + "Bash(gh pr list *)", + "Bash(gh pr diff *)", + "Bash(gh pr checks *)", + "Bash(gh issue view *)", + "Bash(gh issue list *)", "Bash(gh run *)", "Bash(git rev-parse *)", "Bash(git show *)", "Bash(git ls-files *)", "Bash(git worktree *)", - "Bash(git push origin loop/*)", - "Bash(git push -u origin loop/*)", - "Bash(gh label *)", "Bash(docker manifest inspect *)", - "Bash(gh release view *)" + "Bash(gh release view *)", + "PowerShell(git status*)", + "PowerShell(git log *)", + "PowerShell(git diff *)", + "PowerShell(git show *)", + "PowerShell(git rev-parse *)", + "PowerShell(git ls-files *)", + "PowerShell(git add *)", + "PowerShell(git commit *)" ], "deny": [ "Read(**/.env)", @@ -67,7 +76,44 @@ "Read(**/*.jks)", "Read(**/*.keystore)", "Read(**/*.p12)", - "Read(**/deny-names.local)" + "Read(**/deny-names.local)", + "Bash(git checkout -f *)", + "Bash(git switch -f *)", + "Bash(git switch --discard-changes *)", + "Bash(git stash drop *)", + "Bash(git stash clear)", + "Bash(git branch -D *)", + "Bash(git tag -d *)", + "Bash(git push --delete *)", + "Bash(git reflog expire *)", + "Bash(git gc --prune=now*)" + ], + "ask": [ + "Bash(git push --force-with-lease*)", + "PowerShell(git push*)", + "Bash(git rebase *)", + "Bash(git cherry-pick *)", + "Bash(git commit --amend*)", + "Bash(git filter-branch *)", + "Bash(gh pr create *)", + "Bash(gh pr edit *)", + "Bash(gh issue create *)", + "Bash(gh issue edit *)", + "Bash(gh pr close *)", + "Bash(gh pr comment *)", + "Bash(gh pr review *)", + "Bash(gh issue close *)", + "Bash(gh issue comment *)", + "Bash(gh release create *)", + "Bash(gh release edit *)", + "Bash(gh release delete *)", + "Bash(gh release upload *)", + "Bash(gh api -X *)", + "Bash(gh api --method *)", + "Bash(gh api * -X *)", + "Bash(gh api * --method *)", + "Bash(gh repo edit *)", + "Bash(gh workflow run *)" ] }, "hooks": { @@ -139,7 +185,8 @@ "hooks": [ { "type": "command", - "command": "which osascript >/dev/null 2>&1 && osascript -e 'display notification \"Claude Code needs your attention\" with title \"Claude Code\"' || which notify-send >/dev/null 2>&1 && notify-send 'Claude Code' 'Claude Code needs your attention' || true" + "command": "$CLAUDE_PROJECT_DIR/.claude/hooks/notify.sh", + "timeout": 5000 } ] } diff --git a/.claude/skills/audit-scan/SKILL.md b/.claude/skills/audit-scan/SKILL.md deleted file mode 100644 index 0ed283d..0000000 --- a/.claude/skills/audit-scan/SKILL.md +++ /dev/null @@ -1,115 +0,0 @@ ---- -name: audit-scan -description: The manager half of the audit and bug-fix loop. Audits one dimension of Solverr's own code per run, tries hard to refute every candidate defect before believing it, and files the survivors as labeled GitHub issues with every affected site enumerated, so the worker loop has a queue. Triage only, it has no file-writing tools and never creates a branch. Use on a schedule, after a release, or when you want the known problems itemised rather than argued about. -argument-hint: "[--dry-run] [dimension] (omit to pick the least recently audited dimension)" -disable-model-invocation: true -allowed-tools: - - Bash(git *) - - Bash(gh issue *) - - Bash(gh label *) - - Bash(uv run *) - - Read - - Glob - - Grep ---- - -Audit one dimension of Solverr per run and turn each surviving defect into one issue the worker loop can pick up. - -This is the sibling of `/port-scan`. Same contract, different input: `/port-scan` reads what upstream did, this reads what Solverr does. Like its sibling it triages only, **has no Edit or Write tool by design**, and never creates a branch. - -## The rule this skill exists to enforce - -**A finding that has not survived an attempt to refute it is a guess.** This project has already paid for the alternative. An audit reported `postform.py`'s percent-encoding as double-encoding, the reasoning was clean, and it was wrong: the form travels as a `data:text/html,` URL, so the browser URL-decodes before the HTML parser runs, and removing `quote()` broke POST for any value holding `%` or `#`. It took a live A/B against an echo service to find that out, after the "bug" had already been written up. - -So the standing rule, also recorded in the `measure-before-fixing-odd-code` memory: **code that looks wrong here usually encodes a measured constraint.** Two consequences, both binding: - -- A finding about code carrying a WHY comment, a `CLAUDE.md` note, or a ledger entry must say **why that recorded reason no longer holds**. If it cannot, it is not a finding. -- A finding whose proof needs a live browser is not confirmable here. File it as `loop:needs-human` with the A/B that would settle it, and let a person run `/live-check`. - -## Arguments - -- `--dry-run` prints every issue it would file, in full, and creates nothing. Always dry-run a dimension the loop has not audited before. -- A dimension name narrows the run. Omitted picks the least recently audited one, judged from the `source:audit` issue history. - -## Step 1: Pick one dimension - -One per run. A sweep over everything produces shallow findings in all of it, and the loop runs often enough that rotation covers the ground. - -| Dimension | Scope | -|---|---| -| `engines` | `src/engines/`, and specifically whether the two engines still agree. A defect in one usually has a twin in the other. | -| `sessions` | `src/sessions.py`, `src/session_reaper.py`, `src/async_runtime.py`: lifecycle, the reaper, `in_use` counting, event-loop boundaries. | -| `contract` | `src/dtos.py`, `src/flaresolverr_service.py`: request validation, error shape, status codes, `/v1` byte compatibility, the ready banner. | -| `passthrough` | `src/passthrough.py`: routing, host allowlisting, in-flight slots, content types. | -| `config` | `src/config.py`, `src/geo.py`: env parsing, defaults, what happens when a value is absent or malformed. | -| `packaging` | `Dockerfile`, `.dockerignore`, `requirements.txt`, the workflows: build context, pins, what reaches the image. | -| `resources` | Anything that can leak: browsers, contexts, temp files, in-flight slots, unbounded growth (`maxTimeout` has no upper bound, Prometheus labels by domain are unbounded cardinality). | - -## Step 2: Read the state before judging anything - -1. **Existing issues, open and closed**, which is where prior verdicts live: - ``` - gh issue list -R unseensnick/Solverr --state all --label source:audit --limit 100 \ - --json number,title,state,labels - ``` - A closed issue is a decided question. **Read the closing comment before re-filing anything that resembles it**, because a rejected finding re-filed every run is how this loop turns into noise. -2. **`CLAUDE.md`** "Architecture (non-obvious)" and "Key decisions (WHY)". Nearly every entry there is a measured constraint that looks like a bug from the outside. -3. **`docs/dev/upstream-sync.md`** "Deliberately different". Same status: cited, never re-argued. -4. **`Handoff.md`** if present. "What failed" is the list of conclusions that were reached confidently and turned out wrong, and "Next steps" often names inherited issues already known and not yet filed. Filing those is good work; re-deriving them as new discoveries is not. -5. `git log --oneline -20` for what changed recently, since new code is where new defects are. - -## Step 3: Find candidates - -Read the dimension's files directly. Look for the things that are actually true here rather than a generic checklist: disagreement between the two engines, a resource acquired on one path and released on another, an `except` that swallows the case it was written for, a value from the request reaching a browser or a shell without validation, a default that is safe on one engine and not the other, an unbounded accumulation. - -For each candidate, before it is allowed to become a finding, **enumerate every site**. List each hit with `file:line`, and record the search itself so the worker can re-run it. A candidate with one known site and no search behind it is not ready to file. - -**Search recursively from `src/`, never `src/*.py`.** A glob stops at the top level and silently skips `src/engines/` and `src/bottle_plugins/`, which is where half the interesting code lives. That mistake produced a false finding on the first run of this skill: `end_use` looked like it was never called anywhere, because the only caller is `src/engines/chrome_engine.py:73`. Use `grep -rn '' src/ --include=*.py`, or the `Grep` tool with a directory path, and confirm the hit count is plausible before trusting a zero. - -## Step 4: Refute, then file what survives - -This is the gate, and it is the reason this skill is worth running. - -For every candidate, argue the other side as hard as you argued the first: find the comment, commit message, ledger entry, or test that would make it correct as written. `git log -S` and `git blame` on the line are the fastest route to the reason. Then: - -- **Refuted**: drop it. If it is a candidate someone would plausibly re-raise, file it as a closed issue recording the reason, so the next run does not spend the same tokens. -- **Confirmed, provable without a browser**: prove it. A failing case in the browser-free suite, or a `uv run` snippet that demonstrates it. Attach the proof to the issue. -- **Confirmed, needs a live browser**: file it, label `loop:needs-human`, and write the exact A/B that would settle it. -- **Still unsure**: `loop:needs-human`. Uncertainty is a fine thing to report and a bad thing to hide. - -## Step 5: File the issues - -One issue per defect, never one per site. Title names the defect and the dimension: - -``` -audit(sessions): the reaper can close a driver a request is still holding -``` - -Body, in this order: - -1. **The defect**, one sentence, stated as what goes wrong rather than what looks odd. -2. **Failure scenario**: concrete inputs or state, then the wrong output. If you cannot write one, you have a smell and not a defect. -3. **Every affected site**, as a checklist with `file:line`, plus the search that produced it. This is the section that stops the worker at four of five sites. -4. **The refutation attempt**: what you looked for that would have made it correct, and why that did not hold. A finding without this section has not been through step 4. -5. **Proof**, or the A/B that would produce it. -6. **Suggested fix**, including whether the proper fix needs a refactor. Say so plainly if it does; `.claude/rules/code-quality.md` prefers the proper fix over threading a workaround through the existing shape. -7. **Risk**: which tripwire zones it touches, if any. - -Label every issue `source:audit`, plus `loop:ready` or `loop:needs-human`. The eligibility rules are the same as `/port-scan`'s and are not restated here: full scope enumerated, covered by the browser-free suite or provably inert to solving, no ledger divergence in scope, no change to the `/v1` shape or the ready banner, and `loop:needs-human` for the widget path, the budget split, `postform.py`'s `quote()` calls, session lifecycle, `geo.py`, dependency pins, or anything you are not certain about. - -No site names in an issue title or body. Issues are a public surface. - -## Step 6: Report - -The dimension audited, candidates found, how many were refuted and by what, issues filed with numbers and labels, and anything skipped as already decided. **A run that refutes everything it found is a successful run**, and worth saying plainly, because the alternative is a loop that manufactures findings to look productive. - -Do not update any doc. This skill files issues and nothing else. - -## Rules - -- One dimension per run. One issue per defect, with all its sites. -- Refute before believing. A finding that skipped step 4 does not get filed. -- Recorded reasons (WHY comments, `CLAUDE.md`, the ledger) are cited and answered, never ignored. -- Anything needing a live browser to prove goes to a human. This skill has no Docker and no browser. -- No Edit, no Write, no branches, no PRs. Triage only. -- No em dashes. Commas, parentheses, periods, colons. diff --git a/.claude/skills/code-research/SKILL.md b/.claude/skills/code-research/SKILL.md new file mode 100644 index 0000000..d11a671 --- /dev/null +++ b/.claude/skills/code-research/SKILL.md @@ -0,0 +1,97 @@ +--- +name: code-research +description: Fan-out research over Solverr to answer a broad question that spans many files, such as where the test gaps are, how a request flows end to end through the controller, spine and engines, where untrusted input reaches a browser, or what one surface still duplicates between the engines. Parallel read-only agents gather file:line-cited findings, the high-stakes claims are adversarially verified against current code, and the result is one prioritized report. Use for questions across many files and modules. Not for a one-file lookup, and not for pre-planning one concrete task, which is /scout. Never edits files. +argument-hint: " (e.g. 'where are the browser-free test gaps', 'how does a request reach a clearing core', 'what does each engine still do on its own')" +disable-model-invocation: false +allowed-tools: + - Bash(git *) + - Bash(uv run *) + - Read + - Glob + - Grep + - Agent +--- + +Answer the research question in `$ARGUMENTS` by fanning out read-only exploration, verifying what comes back against current code, and writing one report cited by `file:line`. **This skill never edits files and never writes code**, as a rule it follows: acting on the report (tests, fixes, refactors) is a separate step the owner approves. + +## When to use this + +- Broad questions over many files or a whole subsystem: "where are the browser-free test gaps", "how does a `/v1` request travel from the controller through the spine to a clearing core", "where does request input reach a browser", "what is still written once per engine". +- Audits where you want coverage and confidence together, not a quick pointer. + +Not for a single fact or a one-file lookup (read it), and not for pre-planning one task such as a port or a fix (`/scout` is lighter and task-scoped). If invoked for something trivial, push back once and name the lighter tool. + +## Standing defaults (no need to ask for these) + +- **Depth.** Open the files and verify each claim against current code. Never pattern matching, never inference from a symbol name. A plausible mechanism that was not read is not a finding. +- **Completeness.** Keep researching until every unknown is resolved, or surfaced as an open question. Do not smooth a gap over to finish the report. +- **Adversarial verification is mandatory**, and it covers claims from `Handoff.md`, memories, docs, and subagents alike. +- **Output format** follows [.claude/rules/plan-output.md](../../rules/plan-output.md), including its word cap. + +## Step 1: Scope and clarify + +Echo the question in one sentence so the owner can correct a misread before agents spend tokens. If it is vague (which surface, which engine, `/v1` or the passthrough, what counts as a gap), use `AskUserQuestion` to narrow it. Decide the angle of decomposition (by surface, by request flow, by concern) and the inclusion bar (what is a finding and what is noise). + +## Step 2: Map the terrain on the main thread + +Cheap reads first, so the agents are briefed well and do not rediscover the obvious: + +1. **`.claude/rules/engine-layer.md`, its seam-depth table.** It says which surfaces are taken over (the spine: `src/assembly.py`, `src/pipeline.py`, `src/budget.py`, `src/sessions.py`, `src/config.py`, `src/dtos.py`) and which stay per engine (each clearing core). The two fail differently: a taken-over surface drops upstream behaviour, a mechanism-depth surface restates one rule at two sites. That decides what the agents look for. +2. **`docs/dev/upstream-sync.md`.** Its "Deliberately different" list and recorded "not applicable" verdicts are not findings unless their reasoning no longer holds. +3. **`CLAUDE.md`** "Architecture (non-obvious)" and "Key decisions (WHY)". Code that looks wrong here usually encodes a measured constraint recorded there. +4. **`Handoff.md`** if present: deferred work, the parked list, and "What failed". The most common research failure is flagging deferred work as missing. +5. `git log --oneline -20` for the recent shape, and for test questions the existing inventory (`src/test_*.py`, with `src/test_engine_conformance.py` as the both-engines rung). + +Write down, verbatim, the already-covered and out-of-scope lists to hand to every agent. + +## Step 3: Fan out parallel explorers + +Split the question into 3 to 6 independent areas and spawn one `Agent` (`subagent_type: Explore`) per area, all in one message. Brief each as a colleague who has not seen this conversation: + +- The goal and the inclusion bar. +- Exact paths, and the already-covered and out-of-scope lists. +- **Search recursively from `src/`**, never `src/*.py`: a top-level glob skips `src/engines/` and `src/bottle_plugins/` and reports a confident zero. +- A `file:line` for every concrete claim, a High / Medium / Low rating, and one line on why each finding matters. +- About 500 to 700 words, ending with an "uncertain, would need to confirm" section. + +The sibling clones `../FlareSolverr` and `../Byparr` are read-only reference. Hand them over when the question has an upstream side, and never check out, pull, or edit anything there. + +## Step 4: Adversarially verify + +Explore agents locate and summarize; they do not verify. Re-read current code yourself for the highest-stakes, most surprising, or most bug-like claims. Typical kills: + +- "X is untested" when a test exists; "X does not exist" when it moved or was renamed. +- A flagged bug the surrounding code already handles, or that cannot be reached. +- A difference from upstream that the ledger records as deliberate. +- A finding on the deferred list from Step 2. +- Misjudged testability: "add a unit test" for logic that only a live challenge can exercise. + +**Library behaviour is cited from the installed source**, under `.venv/Lib/site-packages/`, never from memory of the docs. Check the package's `.dist-info` version against `requirements.txt` first, since the local `.venv` can lag the pin. + +Label each surviving finding **verified** (you re-read it) or **reported** (agent-cited, not re-read). Anything that would change the conclusion if wrong must be verified. On an exhaustive question, verification can fan out too, one skeptic per top finding, each told to refute it. + +## Step 5: Synthesize the report + +One report into the conversation, not a file unless asked, in the structure from [plan-output.md](../../rules/plan-output.md): headline, findings graded High / Medium / Low, stale docs, the plan if the research feeds implementation, open questions. + +- **Dedupe across agents.** Two agents finding the same thing is one finding. +- **Deferred work is not a defect.** Say so once and move on. +- **Say how each finding can be proven**: the browser-free suite, or only `/live-check`. A claim about whether a page still clears cannot come from the unit tests. +- **A rule that must hold for both engines** is reported with both engines' sites, since `engine-layer.md` fixes it for both in one commit. + +## Step 6: Hand off + +End with **"Ready to act"** and the recommended first batch, or **"Open questions block this."** Then stop. Do not start writing tests, fixes, or refactors. + +## Scale + +Verification is never the knob. Breadth is: a focused question gets one fan-out, "be exhaustive" gets further rounds until one surfaces nothing new, plus a completeness pass asking which area or angle was not covered. + +## Rules + +- Read-only. Never edits, never writes code, never starts the fix. +- Every concrete claim cites `file:line` from current code. Memory, `Handoff.md`, doc and agent claims are hypotheses until cited. +- Surface stale memories and docs found along the way instead of acting on them. +- Never fill an unresolved gap with an assumption. Research it or surface it as an open question. +- No interim narration: one sentence when the fan-out starts, then nothing until the report. +- No em dashes. Commas, parentheses, periods, colons. diff --git a/.claude/skills/deep-audit/SKILL.md b/.claude/skills/deep-audit/SKILL.md new file mode 100644 index 0000000..0d050c4 --- /dev/null +++ b/.claude/skills/deep-audit/SKILL.md @@ -0,0 +1,160 @@ +--- +name: deep-audit +description: Read-only, many-agent audit of a whole range of work (default the current branch against main) for bugs, misconfiguration, missing wiring, dead code, rule violations, and drift between the Chrome and stealth engines. Runs a Workflow that slices the range, points fourteen lenses at it (including two-ends tracing, where independent agents start at the write side and the read side of one value and a third compares them), refutes every finding before it counts, mutation-checks the range's new tests, and reports. Never edits code; the owner decides what gets fixed. Use when the user asks for a broad audit, a pre-release or pre-PR audit, or "check that all the changes work together". NOT for one diff (/pr-review), drift against FlareSolverr or Byparr (/upstream-audit), or one concrete task (/scout). +argument-hint: "[.. | | ] [--lenses a,b] [--no-mutate] [--yes]" +disable-model-invocation: false +allowed-tools: + - Bash(git status*) + - Bash(git log *) + - Bash(git diff *) + - Bash(git merge-base *) + - Bash(git rev-parse *) +--- + +Audit a whole range of Solverr work with many agents, verify every finding adversarially, and report. +**This skill writes one file, the ledger, and nothing else.** It never edits code, never fixes, never +commits. What gets fixed, and how, is the owner's call once the report is in. + +How it differs from its siblings: `/pr-review` is a fast diff check with no verification, +`/upstream-audit` compares Solverr against its two upstreams, `/scout` plans one task. This one is +broad and slow on purpose, and the map step shows what it will cost before anything expensive runs. + +The engine is [deep-audit.workflow.js](deep-audit.workflow.js), run through the `Workflow` tool with +`scriptPath`. Invoking this skill is the opt-in to multi-agent orchestration. A default run spawns far +more agents than the session's workflow-size guideline (about 15); if the owner wants that guideline +raised to match, it is "Dynamic workflow size" in the app's config. + +## Arguments + +- **No scope**: the current branch against `main` (`git merge-base main HEAD` to `HEAD`). +- **`..`**: that commit range. +- **A path**: the default range, limited to files under that path. +- **A surface name** (`sessions`, `result assembly`, `passthrough`): resolve it to paths through the + seam-depth table in `.claude/rules/engine-layer.md` and "Where things live" in `CLAUDE.md`, then + treat it as a path scope. If it resolves to nothing, ask. +- **`--lenses a,b`**: run only these lenses (names below). The critic stays inside the same set. +- **`--no-mutate`**: skip the mutation step. It is on by default because the browser-free suite runs in + seconds here, and `engine-layer.md` requires every new test to have been seen failing. +- **`--yes`**: skip the approval stop after the map. + +## The lenses + +| Lens | Target | What it hunts | +|---|---|---| +| `correctness` | each code slice (`code-reviewer`) | Logic, `None` and empty handling, state, error handling, and whether changed functions still fit their callers | +| `security` | each code slice (`security-reviewer`) | Untrusted `/v1` and passthrough input reaching a browser, secrets and solved cookies in logs, credentials left on disk | +| `performance` | each code slice (`performance-reviewer`) | Browser launches, the `maxTimeout` budget, blocking the stealth loop, lock scope, unbounded growth | +| `concurrency` | each code slice | Waitress threads racing on shared state, the reaper against a session in use, locks held across I/O, floating tasks on the stealth loop, cleanup outside `finally` | +| `dead` | each code slice | Unused code, settings, env vars and branches, with a mandatory whole-tree re-search behind every "unused" | +| `rules` | each rule file over groups of slices | `engine-layer`, `code-quality`, `architecture`, `error-handling`, `security`, `testing`, checked line by line | +| `docs` | each docs slice (`doc-reviewer`) | Claims in the README, CHANGELOG, `CLAUDE.md`, rules, skills and `docs/dev` that current code contradicts, with counts re-derived | +| `tests` | each tests slice | Tests that cannot fail, over-claiming names, per-engine pairs that should be one conformance case, and a reasoned mutation of up to five | +| `tooling` | each tooling slice | Git and Claude Code hooks, CI workflows, settings and permissions: regexes that cannot match or match too much, Windows paths, rule precedence, CI steps that cannot fail | +| `wiring` | the range, twice | Packaging (Dockerfile, compose, `requirements.txt` against imports, the image's env); config and contract (every env var read against the README table and its default, `/v1` fields declared against fields read) | +| `parity` | user-visible changes, batched | Write-once gaps between the Chrome and stealth engines, and capabilities that silently do nothing on one engine | +| `sibling` | fix commits, batched | The same defect still sitting at another site: the other engine, the other command, the same call shape elsewhere | +| `upstream` | each taken-over surface | Behaviour the replaced FlareSolverr or Byparr code had that the spine lost, walked from theirs | +| `twoends` | each write/read pair | Two blind tracers (write side, read side) and a reconciler that reports every mismatch | + +Every finding carries an evidence label: **executed** when a command's output decided it, **traced** +otherwise. The report keeps the two apart. + +## Step 0: A clean tree and a real range + +Agents read the working tree, while the range is committed history, so uncommitted edits make +them report against code that is not in the range. **If `git status` is dirty, stop and ask** the owner +to commit or stash first. + +Resolve `base` and `head` to short SHAs with `git rev-parse`, and state the range and its size +(`git log --oneline .. | wc -l`, `git diff --shortstat ...`) in one line. + +## Step 1: Ground + +Read these, and pass them to the workflow verbatim as `ground`: + +- **`parked`**: the `## Parked` section of `Handoff.md` (gitignored, on disk), if it has one. Parked + items are not findings. +- **`ledger`**: the table rows in [ledger.md](ledger.md). A finding refuted before is not raised again + while its reason still holds. + +Deliberate divergences live in `docs/dev/upstream-sync.md`, and the engine-layer rulings in +`docs/dev/engine-layer-architecture.md`. The agents are told to read those themselves; don't inline them. + +## Step 2: Map, then stop for approval + +``` +Workflow({ scriptPath: "/.claude/skills/deep-audit/deep-audit.workflow.js", + args: { mode: "map", base, head, pathFilter, lenses, ground } }) +``` + +Four agents build the map: slices (code, docs, tests, tooling), up to 15 write/read pairs ranked by +risk, the fix commits plus the user-visible changes (the `[Unreleased]` entries the range added to +`CHANGELOG.md`, plus `feat` and `fix` commits), and the taken-over surfaces the range touches. The +result includes an estimate of finder agents per lens. + +Show the owner: the slice list with sizes, the pairs, the surfaces, what was excluded and why, and the +estimate (finders, plus roughly one verifier per medium or low finding and three per high one). Then +**stop and wait**, unless `--yes` was passed. The owner may drop slices or pairs, add a pair, or narrow +the range; edit the map object to match. The map decides everything downstream, and this is the last +cheap point to fix it. + +## Step 3: Audit + +``` +Workflow({ scriptPath: "", args: { mode: "audit", base, head, pathFilter, lenses, ground, mutate, map } }) +``` + +`mutate` is `true` unless `--no-mutate` was passed. It runs in the background; wait for the completion +notification. What the script does, so the report can explain it: + +1. **Find.** Every lens over its targets, in parallel. +2. **Verify.** Findings on the same file within three lines merge (lenses overlap), then each is attacked + by skeptics told to refute it, defaulting to refuted. A high finding gets three, each from a + different angle: re-read the code, settle it by running something that changes no tracked file, and + check whether it is ruled, recorded as deliberate, parked, in the ledger or contradicted by a + passing gate. It survives on two of three. A medium or low finding gets one skeptic doing all three. +3. **Critic.** One agent reads the coverage table and names up to ten gaps. Those gaps get one round of + finders and verification; there is no second round, and the report says so. +4. **Mutate.** One agent in a throwaway worktree deletes the production clause each of up to five new + tests names, runs that test through the browser-free suite, and reports any that stay green. + +If the run dies part-way, resume it with `resumeFromRunId` and the same args. Finished agents replay +from cache. + +## Step 4: Report + +In the conversation, in the structure of [plan-output.md](../../rules/plan-output.md): headline, +findings graded High / Medium / Low, stale docs, open questions. There is no plan section, since this is +an audit. The 700-word cap is waived for the findings list only, because a range audit's length scales +with its range. Everything around the list stays dense. + +- **Each finding**: bolded one-line claim, `file:line`, the lenses that raised it, the evidence label. + High findings get the evidence prose; medium and low findings are one line. +- **High findings that are still traced** go in their own short list, each with the probe that would + settle it (a browser-free test, a log line, a `/live-check` run). They are not confirmed until + someone runs that. +- **Stale docs** come from the `docs` lens and from any finding whose "defect" is really a doc that + never caught up. +- **Coverage**: a compact table of lens by target with its status. Name every failed task and every + gap the critic raised that was not re-run. Without this, "no findings" can't be told apart from + "nobody looked". +- **By surface**: group confirmed findings by surface. A surface with more than about five confirmed + findings is flagged as a design problem for the owner rather than a list of fixes. +- **Refuted**: a count, plus any refutation the owner might want to overturn. + +Say plainly that the run was read-only, and which findings still need a live check. + +## Step 5: The ledger + +Append each refuted finding to [ledger.md](ledger.md) as one row: the range, `file:line`, the claim in +a short phrase, why it was refuted. **Leave the file uncommitted**; committing it is the owner's call. +A row stops holding once its reason stops being true, so a later run may re-raise it with the new +evidence. + +## Rules + +- Read-only. No code edits, no fixes, no commits, no pushes. The ledger is the only write. +- Never skip the approval stop after the map unless `--yes` was passed. +- Never report a finding the verifiers refuted, and never promote a traced high finding to confirmed. +- Never let a zero stand for coverage: a lens that returned nothing is reported with what it searched. +- No em dashes, no AI watermarks, in the report or the ledger. diff --git a/.claude/skills/deep-audit/deep-audit.workflow.js b/.claude/skills/deep-audit/deep-audit.workflow.js new file mode 100644 index 0000000..35d644d --- /dev/null +++ b/.claude/skills/deep-audit/deep-audit.workflow.js @@ -0,0 +1,380 @@ +export const meta = { + name: 'deep-audit', + description: 'Read-only range audit of Solverr: many lenses per slice, two-ends tracing, adversarial verification, mutation checks', + whenToUse: 'Run by the /deep-audit skill: first with mode "map", then with mode "audit" and the approved map', + phases: [ + { title: 'Map', detail: 'slice the range, find write/read pairs, classify commits and surfaces' }, + { title: 'Find', detail: 'every lens over every slice, pair, commit batch and surface' }, + { title: 'Verify', detail: 'skeptics try to refute each deduplicated finding' }, + { title: 'Critic', detail: 'name uncovered ground and audit it once' }, + { title: 'Mutate', detail: 'real mutation checks of new tests in a throwaway worktree' }, + ], +} + +const A = args || {} +if (A.mode !== 'map' && A.mode !== 'audit') throw new Error('args.mode must be "map" or "audit"') +if (!A.base || !A.head) throw new Error('args.base and args.head are required') + +const RANGE = `${A.base}...${A.head}` +const SCOPE = A.pathFilter ? `${RANGE}, limited to ${A.pathFilter}` : RANGE +const GROUND = A.ground || {} +const SUITE = "PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src" + +const RULE_FILES = ['engine-layer.md', 'code-quality.md', 'architecture.md', 'error-handling.md', 'security.md', 'testing.md'] +const RULE_GROUP = 4 +const SIBLING_BATCH = 10 +const PARITY_BATCH = 10 +const CODE_LENSES = ['correctness', 'security', 'performance', 'concurrency', 'dead'] +const ALL_LENSES = ['correctness', 'security', 'performance', 'concurrency', 'dead', 'rules', 'docs', 'tests', 'tooling', 'wiring', 'parity', 'sibling', 'upstream', 'twoends'] + +const PREAMBLE = `You are one agent in a read-only audit of Solverr, a FlareSolverr fork: a Python 3.14 bypass proxy (bottle + waitress, synchronous WSGI) serving the FlareSolverr /v1 API, with two browser engines behind one interface. "chrome" is Selenium with the vendored undetected_chromedriver; "stealth" is Camoufox via invisible_playwright, run on one background asyncio loop thread (src/async_runtime.py). What both engines must do the same way lives once in a shared spine (src/assembly.py, src/pipeline.py, src/budget.py, src/sessions.py); each engine is an adapter over a clearing core derived from its upstream. The current directory is the repo root. Read-only reference clones of both upstreams sit beside it: ../FlareSolverr and ../Byparr. + +The range under audit is ${SCOPE}. Commits whose subject says they sync FlareSolverr or Byparr are ports: audit Solverr's adaptation, not upstream's own choices. + +Hard rules: +- Read-only. Never edit, create, stage or commit a tracked file, never start Docker or a browser, and never run src/tests.py (it launches real browsers against live sites). Python runs only through uv. The browser-free suite is: ${SUITE}. One module is: PYTHONPATH=src uv run --no-project python -m unittest . +- Every finding cites a file:line you read in this run. A claim you cannot cite is not a finding. +- Label evidence honestly. "executed" means a command's output decides the claim (a whole-tree grep, git log or show, a browser-free test run, a small uv run --no-project python -c snippet reproducing pure logic). Everything else is "traced". A traced call chain proves the code exists, not that it does what you claim: follow the data to where it is used, not to the first function that agrees with you. +- Whether a page still clears a real challenge cannot be settled here. That needs /live-check; name it as the probe instead of guessing. +- Not findings: items on the parked list below; anything docs/dev/upstream-sync.md records under "Deliberately different"; a ruling in docs/dev/engine-layer-architecture.md or .claude/rules/engine-layer.md (read them before reporting; a decline whose stated premise no longer holds IS a finding); ledger entries below whose refutation still holds; upstream idiom inside files kept byte-identical to FlareSolverr (src/undetected_chromedriver/, src/tests.py, src/tests_sites.py, html_samples/, src/bottle_plugins/ except prometheus_plugin.py) or inside either clearing core, unless the range changed those lines. +- The project law is CLAUDE.md and .claude/rules/*.md. The seam-depth table in .claude/rules/engine-layer.md says which surfaces are shared and which stay per engine; read it before judging duplication. Code that looks wrong often encodes a measured constraint (.claude/rules/architecture.md lists them): ask for the measurement rather than calling it a bug. +- Tool traps on this machine: the Bash tool turns a double backslash into one before bash sees it, so build literal backslashes in Python with chr(92); working copies may be CRLF while the upstream clones are LF, so diff against them with --strip-trailing-cr; a Docker path argument from Git Bash needs MSYS_NO_PATHCONV=1. A search that returns zero is suspect: re-run it another way before believing it. + +Parked, not findings: +${GROUND.parked || '(none given)'} + +Ledger of previously refuted findings: +${GROUND.ledger || '(empty)'}` + +const LENS_BRIEFS = { + correctness: { agentType: 'code-reviewer', brief: 'Find real correctness defects: wrong logic, None and empty handling, bool-versus-int and truthiness on request input, state bugs, error handling that swallows a failure or leaks a traceback into the response, and anything that breaks the /v1 contract (additive optional fields only, the "FlareSolverr is ready!" banner). The changes have to work together, so follow each changed function to its callers inside and outside the slice, and when a change lands in one engine, open the other.' }, + security: { agentType: 'security-reviewer', brief: 'Find security defects per .claude/rules/security.md: untrusted /v1 and passthrough input reaching a browser (only http(s) URLs, postData escaping in postform.py, proxy validation in geo.proxy_to_config, the passthrough host allow list), secrets or solved cookies reaching a log or an error message, proxy credentials left on disk, a default flipped to exposed. Solverr has no auth by design; do not report its absence.' }, + performance: { agentType: 'performance-reviewer', brief: 'Find real bottlenecks: extra browser launches, work that spends the maxTimeout budget twice, anything blocking the single stealth event loop, locks held across I/O or a whole solve, waitress threads held longer than a solve needs, and caches, dicts or label sets with no bound.' }, + concurrency: { brief: 'Hunt ordering and lifecycle bugs: two waitress threads racing on shared module state without a lock; a session found and then used in separate lock acquisitions, so the reaper or the cap can act in between (src/test_session_reaping.py shows the deterministic way to prove a race); a lock held across a socket write or a browser call; a coroutine not awaited, or a task nobody holds, on the stealth loop; AsyncRuntime.run called from the loop thread itself; a deadline computed on one clock and compared on another; a browser, context, page or temp directory that leaks when a launch or solve fails part-way (cleanup belongs in finally). Name the interleaving that breaks and what the client sees.' }, + dead: { brief: 'Find dead and unused code the range added or left behind: unused functions, parameters, constants, environment variables and config readers, branches that can no longer be reached, commented-out blocks. Before calling anything unused, search the WHOLE repository, including src/test_*.py, src/engine_fakes.py, .github/, .githooks/, .claude/, Dockerfile, docker-compose.yml, README.md and docs/; bottle route and plugin decorators, getattr, and names read from the environment all count as uses. "Nothing calls X" is the claim class that fails most, so put exactly what you searched in evidenceDetail.' }, + rules: { brief: 'Audit against one rule file. Read it first and turn it into a checklist, then walk the slices. Each finding quotes a few words of the exact rule it breaks. Rules a hook already enforces (em dashes, commit message shape, site names in docs) are findings only where the hook cannot see them.' }, + docs: { agentType: 'doc-reviewer', brief: 'Check every claim in the docs in this slice against current code: files, symbols, environment variables and their defaults, counts, behaviour. Re-derive any stated count rather than trusting it. Also check claims in CLAUDE.md, .claude/rules and .claude/skills about code or tooling the range changed, and that CONTRIBUTING.md and .github/pull_request_template.md still match .claude/rules/workflow.md.' }, + tests: { brief: 'Audit the tests in this slice: tests that cannot fail (asserting the value just set, asserting a mock was called without checking its arguments, a loop that never iterates), names that claim more than the body checks, a rule for both engines pinned by a hand-maintained per-engine pair instead of one case in src/test_engine_conformance.py, and a rule for both engines pinned on only one. For up to five new tests, do a reasoned mutation: name the production clause the test pins and say whether deleting it would turn the test red. If it would not, that is a finding, labelled traced.' }, + tooling: { brief: 'Audit the repository tooling in this slice: .githooks/ scripts and their self-test, .claude/hooks/ guards and their fixtures, .claude/settings.json permissions and hook wiring, and .github/workflows/. Look for a regex that cannot match what it claims (Windows backslash paths, PowerShell spellings, case) or that matches far more than intended, a guard or rule with no fixture that would fail without it, permission rules where deny-then-ask-then-allow precedence gives a different answer than the comment intends, a CI step that cannot fail, an action not pinned to a commit, and docs (CLAUDE.md, .claude/rules/workflow.md, CONTRIBUTING.md) that describe the tooling differently from what it does. Running bash .githooks/tests/run.sh or bash .claude/hooks/tests/run-all.sh is allowed and counts as executed evidence; the second takes about 90 seconds on this machine, so give it a long timeout.' }, + wiring: { brief: '' }, + parity: { brief: 'For each user-visible change below, check that the Chrome and stealth engines both got it in this range, per "Write once, both engines get it" in .claude/rules/engine-layer.md. A gap is a finding unless it falls under a recorded exit: a named browser-automation mechanism one engine genuinely lacks, cited in the commit and recorded in docs/dev/upstream-sync.md, or the clearing-core decline for a knob or dependency only one core uses. Also report a capability that silently does nothing on one engine instead of being routed or refused by name, and a per-engine branch or a new boolean-flag combination inside the shared spine.' }, + sibling: { brief: 'Each commit below fixed a defect. For each one, read its diff, state the defect as a pattern, then search the whole tree for other sites with the same pattern: the other engine\'s adapter or clearing core, the other /v1 command, the passthrough, the same call shape elsewhere. Report every site that still carries the defect. A site the commit message or docs/dev/upstream-sync.md deliberately left, with a reason, is not a finding.' }, + upstream: { brief: 'This surface was taken over from an upstream into Solverr\'s shared spine. Walk the replaced upstream code\'s behaviour end to end, starting from theirs rather than ours: ../FlareSolverr for the Chrome side, the controller and sessions; ../Byparr for the stealth side. Mark each behaviour present (cite ours), deliberately dropped (cite docs/dev/upstream-sync.md or docs/dev/engine-layer-architecture.md) or missing. Report only the missing ones as findings.' }, + twoends: { brief: '' }, +} + +const WIRING_TASKS = [ + { target: 'packaging', brief: 'Audit packaging for the range: the Dockerfile (base image, installed packages, the Camoufox fetch, the baked geoip database and STEALTHFOX_GEOIP_MMDB, HEALTHCHECK, the user and paths the code expects), docker-compose.yml, a .dockerignore entry excluding something the image needs, requirements.txt pins against what src/ imports and against each other (one pin capping another), test-requirements.txt, the package.json version, and the release workflows that build and publish the image.' }, + { target: 'config and contract', brief: 'Audit config and contract wiring for the range: every environment variable src/ reads (config.py, utils.py, flaresolverr.py, and anywhere else os.environ appears) against the README configuration table, its default in code against the default the README states and docker-compose.yml sets, and variables documented but never read; /v1 request fields declared in src/dtos.py against the fields the controller and engines read; response fields emitted against the README response example; error messages clients or the fallback match on; and the CHANGELOG [Unreleased] entries against what the range actually changed.' }, +] + +const SEV = { type: 'string', enum: ['high', 'medium', 'low'] } +const FINDINGS = { + type: 'object', + properties: { + findings: { + type: 'array', + items: { + type: 'object', + properties: { + title: { type: 'string', description: 'the defect as a one-line claim' }, + file: { type: 'string', description: 'repo-relative path' }, + line: { type: 'integer' }, + severity: SEV, + claim: { type: 'string', description: 'the defect in two or three sentences' }, + failureScenario: { type: 'string', description: 'concrete request, config or state, and the wrong result a client or deployer sees' }, + evidence: { type: 'string', enum: ['executed', 'traced'] }, + evidenceDetail: { type: 'string', description: 'the command and its output, or the lines read' }, + surface: { type: 'string', description: 'surface or subsystem, e.g. "sessions", "result assembly", "passthrough", "tooling"' }, + }, + required: ['title', 'file', 'line', 'severity', 'claim', 'failureScenario', 'evidence', 'evidenceDetail', 'surface'], + }, + }, + searched: { type: 'string', description: 'what you covered, so an empty result is distinguishable from an unsearched one' }, + }, + required: ['findings', 'searched'], +} + +const CONTRACT = { + type: 'object', + properties: { + sites: { type: 'array', items: { type: 'string' }, description: 'file:line of every site on your side' }, + key: { type: 'string', description: 'exact name, key, field, env var or message shape' }, + type: { type: 'string' }, + unitsAndScale: { type: 'string', description: 'e.g. ms or s, bytes, 0-based index, a list or a dict' }, + defaultAndEmpty: { type: 'string', description: 'default value, and what None, empty or missing means' }, + timing: { type: 'string', description: 'when this side acts relative to the request, the solve, the lock, and the other side' }, + otherParties: { type: 'string', description: 'every other writer or reader you found, including the other engine' }, + notes: { type: 'string' }, + }, + required: ['sites', 'key', 'type', 'unitsAndScale', 'defaultAndEmpty', 'timing', 'otherParties', 'notes'], +} + +const VERDICT = { + type: 'object', + properties: { + refuted: { type: 'boolean' }, + reason: { type: 'string' }, + evidence: { type: 'string', enum: ['executed', 'traced'] }, + probe: { type: 'string', description: 'if not settled by execution, the one probe (a browser-free test, a log line, a /live-check run) that would settle it' }, + severity: SEV, + }, + required: ['refuted', 'reason', 'evidence', 'probe', 'severity'], +} + +const chunk = (xs, n) => { + const out = [] + for (let i = 0; i < xs.length; i += n) out.push(xs.slice(i, i + n)) + return out +} +const sliceText = s => `Slice "${s.id}" (${s.title}; surface: ${s.surface}; engines: ${s.engines}). Its changed files: run git diff --name-only ${RANGE} -- ${s.paths.join(' ')}` +const findingsPrompt = (brief, target) => `${PREAMBLE}\n\nYour lens: ${brief}\n\nYour target:\n${target}\n\nReturn every finding that survives your own re-read. An empty list is a valid answer when "searched" says what you covered.` + +// One task per lens and target. Shared by map mode (for the estimate) and audit mode. +function planTasks(map, lenses) { + const on = new Set(lenses && lenses.length ? lenses : ALL_LENSES) + const code = map.slices.filter(s => s.kind === 'code') + const tasks = [] + const add = (lens, target, prompt, agentType) => tasks.push({ lens, target, prompt, agentType }) + for (const lens of CODE_LENSES) { + if (!on.has(lens)) continue + for (const s of code) add(lens, s.id, findingsPrompt(LENS_BRIEFS[lens].brief, sliceText(s)), LENS_BRIEFS[lens].agentType) + } + if (on.has('rules')) { + for (const rule of RULE_FILES) { + for (const group of chunk(code, RULE_GROUP)) { + add('rules', `${rule}: ${group.map(s => s.id).join(', ')}`, + findingsPrompt(`${LENS_BRIEFS.rules.brief} The rule file is .claude/rules/${rule}.`, group.map(sliceText).join('\n'))) + } + } + } + if (on.has('docs')) for (const s of map.slices.filter(x => x.kind === 'docs')) add('docs', s.id, findingsPrompt(LENS_BRIEFS.docs.brief, sliceText(s)), 'doc-reviewer') + if (on.has('tests')) for (const s of map.slices.filter(x => x.kind === 'tests')) add('tests', s.id, findingsPrompt(LENS_BRIEFS.tests.brief, sliceText(s))) + if (on.has('tooling')) for (const s of map.slices.filter(x => x.kind === 'tooling')) add('tooling', s.id, findingsPrompt(LENS_BRIEFS.tooling.brief, sliceText(s))) + if (on.has('wiring')) for (const w of WIRING_TASKS) add('wiring', w.target, findingsPrompt(w.brief, `The whole range ${SCOPE}.`)) + if (on.has('parity')) { + for (const batch of chunk(map.userVisibleChanges, PARITY_BATCH)) { + add('parity', batch.map(c => c.summary).join(' | ').slice(0, 120), + findingsPrompt(LENS_BRIEFS.parity.brief, batch.map(c => `- ${c.summary} (source: ${c.source}; engines: ${c.engines})`).join('\n'))) + } + } + if (on.has('sibling')) { + for (const batch of chunk(map.fixCommits, SIBLING_BATCH)) { + add('sibling', batch.map(c => c.sha).join(','), + findingsPrompt(LENS_BRIEFS.sibling.brief, batch.map(c => `- ${c.sha} ${c.subject}`).join('\n'))) + } + } + if (on.has('upstream')) { + for (const s of map.surfaces) { + add('upstream', s.surface, findingsPrompt(LENS_BRIEFS.upstream.brief, + `Surface: ${s.surface}. Spine files: ${s.spineFiles.join(', ')}. Replaced upstream code: ${s.replacedUpstream.join(', ')}. Record: ${s.record}`)) + } + } + if (on.has('twoends')) for (const p of map.pairs) tasks.push({ lens: 'twoends', target: p.id, pair: p }) + return tasks +} + +function countAgents(tasks) { + return tasks.reduce((n, t) => n + (t.lens === 'twoends' ? 3 : 1), 0) +} + +// ---------------------------------------------------------------- map mode + +if (A.mode === 'map') { + phase('Map') + const mapBase = `${PREAMBLE}\n\nYou are building the audit's map, not auditing. Be complete: whatever you leave out goes unaudited.` + const [sl, pr, cm, su] = await parallel([ + () => agent(`${mapBase}\n\nSplit the files changed in ${SCOPE} (git diff --name-only ${RANGE}${A.pathFilter ? ' -- ' + A.pathFilter : ''}) into cohesive slices by subsystem, not by file. Kinds: "code" (Python under src/ other than tests), "tests" (src/test_*.py and src/engine_fakes.py), "docs" (markdown, including .claude/rules, .claude/skills, .claude/agents and docs/), "tooling" (.githooks/, .claude/hooks/ scripts and fixtures, .claude/settings.json, .github/workflows/, renovate.json, .gitattributes, .editorconfig, and .claude/skills/*/*.js). Keep each code slice to roughly 40 changed files or 3000 changed lines, and each docs, tests or tooling slice to roughly 30 files. Give each slice repo-relative paths or directories that pathspec-match its files. Say which engines each slice concerns. List under "excluded", each with why: binary assets, html_samples/, the vendored src/undetected_chromedriver/ unless the range changed it, and packaging files (Dockerfile, docker-compose.yml, requirements*.txt, .dockerignore, package.json), since a separate wiring lens covers those.`, { + label: 'map:slices', phase: 'Map', effort: 'high', + schema: { + type: 'object', + properties: { + slices: { type: 'array', items: { type: 'object', properties: { + id: { type: 'string' }, kind: { type: 'string', enum: ['code', 'docs', 'tests', 'tooling'] }, title: { type: 'string' }, + paths: { type: 'array', items: { type: 'string' } }, surface: { type: 'string' }, + engines: { type: 'string', enum: ['chrome', 'stealth', 'both', 'neutral'] }, approxFiles: { type: 'integer' }, + }, required: ['id', 'kind', 'title', 'paths', 'surface', 'engines', 'approxFiles'] } }, + excluded: { type: 'array', items: { type: 'object', properties: { what: { type: 'string' }, why: { type: 'string' } }, required: ['what', 'why'] } }, + }, + required: ['slices', 'excluded'], + }, + }), + () => agent(`${mapBase}\n\nFind the write/read pairs this range touches: places where one side produces a value and another consumes it, so two independent tracers can meet in the middle. Look especially at: a /v1 request field parsed and typed in src/dtos.py and read by the controller or an engine; the response assembled in src/assembly.py and serialised to the client; an environment variable read in src/config.py or src/utils.py against the README table and docker-compose.yml; a session created and stored in SessionStore against the reaper and the cap that evict it; cookies set through the pipeline against the cookie jar read back in assembly; the proxy dict validated in geo.proxy_to_config against the browser launch that uses it; the passthrough cache written and read; the timezone and language resolved once in geo.py and handed to both engines; a value logged against what the log may contain. Mark a value that goes out and comes back (serialise and deserialise, set and read back) as "round-trip", and a value both engines produce for one consumer as "cross-engine". Rank by risk; return at most 15.`, { + label: 'map:pairs', phase: 'Map', effort: 'high', + schema: { + type: 'object', + properties: { pairs: { type: 'array', items: { type: 'object', properties: { + id: { type: 'string' }, value: { type: 'string' }, writeSide: { type: 'string', description: 'file:symbol' }, + readSide: { type: 'string', description: 'file:symbol' }, kind: { type: 'string', enum: ['write-read', 'round-trip', 'cross-engine'] }, why: { type: 'string' }, + }, required: ['id', 'value', 'writeSide', 'readSide', 'kind', 'why'] } } }, + required: ['pairs'], + }, + }), + () => agent(`${mapBase}\n\nClassify commits in ${SCOPE}. "fixCommits": every commit that fixed a defect (usually a "fix" subject), with sha and subject. "userVisibleChanges": everything a deployer or an API client can observe that the range changed. The [Unreleased] entries the range added to CHANGELOG.md are the primary source, one per bullet (git diff ${RANGE} -- CHANGELOG.md shows them); add any feat or fix commit that changed observable behaviour without an entry. Say which engines each change claims to cover.`, { + label: 'map:commits', phase: 'Map', effort: 'medium', + schema: { + type: 'object', + properties: { + fixCommits: { type: 'array', items: { type: 'object', properties: { sha: { type: 'string' }, subject: { type: 'string' } }, required: ['sha', 'subject'] } }, + userVisibleChanges: { type: 'array', items: { type: 'object', properties: { + summary: { type: 'string' }, source: { type: 'string' }, engines: { type: 'string', enum: ['chrome', 'stealth', 'both', 'neutral'] }, + }, required: ['summary', 'source', 'engines'] } }, + }, + required: ['fixCommits', 'userVisibleChanges'], + }, + }), + () => agent(`${mapBase}\n\nList the surfaces this range touches that were taken over into the shared spine: the rows the seam-depth table in .claude/rules/engine-layer.md marks Done (request boundary, result assembly, solve orchestration, sessions, config). For each, give the spine files the range touched, the upstream code the spine replaced (file and function in ../FlareSolverr or ../Byparr), and the record that documents the takeover (a section of docs/dev/engine-layer-architecture.md or docs/dev/upstream-sync.md). An empty list is right when the range touches none of them.`, { + label: 'map:surfaces', phase: 'Map', effort: 'medium', + schema: { + type: 'object', + properties: { surfaces: { type: 'array', items: { type: 'object', properties: { + surface: { type: 'string' }, spineFiles: { type: 'array', items: { type: 'string' } }, + replacedUpstream: { type: 'array', items: { type: 'string' } }, record: { type: 'string' }, + }, required: ['surface', 'spineFiles', 'replacedUpstream', 'record'] } } }, + required: ['surfaces'], + }, + }), + ]) + if (!sl || !pr || !cm || !su) throw new Error('a map agent failed; re-run map mode') + const map = { + slices: sl.slices, excluded: sl.excluded, pairs: pr.pairs, + fixCommits: cm.fixCommits, userVisibleChanges: cm.userVisibleChanges, surfaces: su.surfaces, + } + const tasks = planTasks(map, A.lenses) + const byLens = {} + for (const t of tasks) byLens[t.lens] = (byLens[t.lens] || 0) + (t.lens === 'twoends' ? 3 : 1) + return { map, estimate: { finderAgents: countAgents(tasks), byLens, note: 'verification adds about one agent per medium or low finding and three per high one, plus one critic and, unless --no-mutate, one mutation agent' } } +} + +// ---------------------------------------------------------------- audit mode + +if (!A.map) throw new Error('audit mode needs args.map from an approved map run') + +async function runTwoEnds(p) { + const side = (which, start) => agent(`${PREAMBLE}\n\nTwo-ends tracing. You own ONE end of a value that crosses a boundary; another agent owns the other end and neither sees the other. Value: ${p.value}. Start at the ${which} side: ${start}. Trace where the value is ${which === 'write' ? 'produced and written' : 'read and consumed'} and document the contract as your side sees it. Do not read the other side's code beyond finding its name.`, { + label: `twoends:${p.id}:${which}`, phase: 'Find', schema: CONTRACT, + }) + const [w, r] = await parallel([() => side('write', p.writeSide), () => side('read', p.readSide)]) + if (!w || !r) return null + return agent(`${PREAMBLE}\n\nReconcile two independent traces of one value (${p.value}, ${p.kind}). Writer's contract:\n${JSON.stringify(w, null, 2)}\n\nReader's contract:\n${JSON.stringify(r, null, 2)}\n\nEvery mismatch is a candidate: a key written but never read or read under another name, different defaults, different units or scale, empty meaning different things, a reader that can run before the writer or outside the lock the writer holds, a second writer one side does not know about (the other engine counts), a field that goes out and does not come back. Re-read the cited lines before reporting each one.`, { + label: `twoends:${p.id}:reconcile`, phase: 'Find', schema: FINDINGS, + }) +} + +async function runTask(t) { + try { + const out = t.lens === 'twoends' + ? await runTwoEnds(t.pair) + : await agent(t.prompt, { label: `${t.lens}:${t.target}`.slice(0, 80), phase: 'Find', schema: FINDINGS, agentType: t.agentType }) + return { t, out } + } catch (e) { + return { t, out: null } + } +} + +const RANK = { high: 3, medium: 2, low: 1 } +function dedupe(findings) { + const out = [] + for (const f of findings) { + const m = out.find(o => o.file === f.file && Math.abs(o.line - f.line) <= 3) + if (!m) { out.push({ ...f, lenses: [f.lens], claims: [f.claim] }); continue } + if (!m.lenses.includes(f.lens)) m.lenses.push(f.lens) + m.claims.push(f.claim) + if (RANK[f.severity] > RANK[m.severity]) m.severity = f.severity + if (f.evidence === 'executed') m.evidence = 'executed' + } + return out +} + +function collect(results, coverage) { + const found = [] + for (const r of results) { + if (!r) continue + coverage.push({ lens: r.t.lens, target: r.t.target, status: r.out ? 'ok' : 'failed', findings: r.out ? r.out.findings.length : 0, searched: r.out ? r.out.searched : '' }) + if (r.out) for (const f of r.out.findings) found.push({ ...f, lens: r.t.lens }) + } + return found +} + +const VERIFY_LENSES = { + code: 'Re-read the cited lines and enough surrounding code to decide whether the defect is real as stated. Check the callers and the data actually flowing in, and for an engine change, what the other engine does.', + execute: `Settle it by running something that changes no tracked file: a whole-tree grep, git log or show, a browser-free test module, a uv run --no-project python -c snippet reproducing the logic, or a hook self-test (bash .githooks/tests/run.sh, bash .claude/hooks/tests/run-all.sh). If only a live browser could settle it, do not refute on that ground; set evidence to traced and name /live-check as the probe.`, + ruled: 'Decide whether this is recorded as deliberate in docs/dev/upstream-sync.md, ruled in docs/dev/engine-layer-architecture.md or .claude/rules/engine-layer.md (including the clearing-core decline and the one-boolean allowance), on the parked list, in the ledger with a reason that still holds, intended per an owner ruling in .claude/rules, or contradicted by a gate that passes (the browser-free suite, bash .githooks/tests/run.sh, bash .claude/hooks/tests/run-all.sh). Any of those refutes it.', +} +const ALL_VERIFY = Object.values(VERIFY_LENSES).join(' ') + +async function verify(f) { + const subject = `Finding (from lenses: ${f.lenses.join(', ')}; severity ${f.severity}):\n${f.title}\n${f.file}:${f.line}\nClaims:\n${f.claims.map(c => '- ' + c).join('\n')}\nFailure scenario: ${f.failureScenario}\nFinder's evidence (${f.evidence}): ${f.evidenceDetail}` + const ask = lens => agent(`${PREAMBLE}\n\nYou are a skeptic. Try to REFUTE the finding below. Default to refuted=true when uncertain. A negative claim ("nothing calls X", "never read") must be re-searched across the whole repository, tests, tooling and docs included, before you accept it.\n\nYour check: ${lens}\n\n${subject}`, { + label: `verify:${f.file.split('/').pop()}:${f.line}`, phase: 'Verify', schema: VERDICT, effort: f.severity === 'high' ? 'high' : undefined, + }) + const votes = (f.severity === 'high' + ? await parallel(Object.values(VERIFY_LENSES).map(l => () => ask(l))) + : [await ask(ALL_VERIFY)]).filter(Boolean) + if (!votes.length) return { ...f, survives: false, verdicts: [], unverifiable: true } + const holding = votes.filter(v => !v.refuted) + const survives = f.severity === 'high' ? holding.length >= 2 : holding.length === 1 + const executed = holding.some(v => v.evidence === 'executed') + const probe = (holding.find(v => v.probe) || {}).probe || '' + const severity = holding.length ? holding.map(v => v.severity).sort((a, b) => RANK[b] - RANK[a])[0] : f.severity + return { ...f, survives, evidence: executed || f.evidence === 'executed' ? 'executed' : 'traced', probe, severity, verdicts: votes } +} + +phase('Find') +const tasks = planTasks(A.map, A.lenses) +log(`${tasks.length} finder tasks, about ${countAgents(tasks)} finder agents`) +const coverage = [] +let found = collect(await parallel(tasks.map(t => () => runTask(t))), coverage) +const failed = coverage.filter(c => c.status === 'failed') +if (failed.length) log(`${failed.length} finder tasks failed and are reported as uncovered`) + +// Barrier on purpose: lenses overlap (correctness and concurrency flag the same line), so dedupe before paying for verification. +phase('Verify') +let deduped = dedupe(found) +log(`${found.length} raw findings, ${deduped.length} after dedupe`) +let verified = (await parallel(deduped.map(f => () => verify(f)))).filter(Boolean) + +phase('Critic') +const gaps = await agent(`${PREAMBLE}\n\nYou are the completeness critic for this audit. Coverage so far (lens, target, status, findings, what was searched):\n${JSON.stringify(coverage, null, 1)}\n\nSlices in the map:\n${A.map.slices.map(s => `${s.id} (${s.kind}): ${s.paths.join(' ')}`).join('\n')}\n\nExcluded:\n${A.map.excluded.map(e => `${e.what}: ${e.why}`).join('\n')}\n\nConfirmed so far:\n${verified.filter(v => v.survives).map(v => `- ${v.title} (${v.file}:${v.line})`).join('\n') || '(none)'}\n\nName what is missing: a failed or thin task, a slice whose "searched" shows it was skimmed, an exclusion that hides real code, a write/read pair nobody traced, an engine the range changed that no slice or parity task looked at, a subsystem the range changed that no slice owns. Return at most 10 gaps, most important first; an empty list is fine.`, { + label: 'critic', phase: 'Critic', effort: 'high', + schema: { + type: 'object', + properties: { gaps: { type: 'array', items: { type: 'object', properties: { + lens: { type: 'string', enum: ALL_LENSES.filter(l => l !== 'twoends' && l !== 'wiring') }, target: { type: 'string', description: 'paths, commits or the behaviour to audit' }, why: { type: 'string' }, + }, required: ['lens', 'target', 'why'] } } }, + required: ['gaps'], + }, +}) +const selected = A.lenses && A.lenses.length ? A.lenses : ALL_LENSES +const gapList = gaps ? gaps.gaps.filter(g => selected.includes(g.lens)) : [] +if (gapList.length) { + log(`critic named ${gapList.length} gaps; auditing them once (no further rounds)`) + const gapTasks = gapList.map(g => ({ + lens: g.lens, target: `gap: ${g.target}`.slice(0, 120), agentType: LENS_BRIEFS[g.lens].agentType, + prompt: findingsPrompt(g.lens === 'rules' ? `${LENS_BRIEFS.rules.brief} Pick the rule file the gap names.` : LENS_BRIEFS[g.lens].brief, `${g.target}\n(Why this was missed: ${g.why})`), + })) + const more = collect(await parallel(gapTasks.map(t => () => runTask(t))), coverage) + found = found.concat(more) + const fresh = dedupe(more).filter(f => !deduped.some(o => o.file === f.file && Math.abs(o.line - f.line) <= 3)) + verified = verified.concat((await parallel(fresh.map(f => () => verify(f)))).filter(Boolean)) +} + +let mutation = null +if (A.mutate) { + phase('Mutate') + const suspects = verified.filter(v => v.survives && v.lenses.includes('tests')).map(v => `${v.file}:${v.line} ${v.title}`) + mutation = await agent(`You work in a throwaway git worktree of the Solverr repo, so edits here never reach the owner's tree. Find the tests added in ${RANGE} (git diff ${RANGE} -- 'src/test_*.py', then the new test methods in those files) and pick up to five, preferring these suspects:\n${suspects.join('\n') || '(none flagged; pick tests whose production clause is easy to isolate)'}\n\nFor each, one at a time: delete or neutralise the production clause the test claims to pin, run only that test from the worktree root with PYTHONPATH=src uv run --no-project python -m unittest .., record red or green, restore the file with git checkout -- , and move on. Python runs only through uv. Never run src/tests.py, which needs a browser. A test that stays green with its clause deleted is a confirmed finding with evidence "executed". If a test cannot run, report that rather than guessing.`, { + label: 'mutate', phase: 'Mutate', isolation: 'worktree', schema: FINDINGS, + }) +} + +const confirmed = verified.filter(v => v.survives).sort((a, b) => RANK[b.severity] - RANK[a.severity]) +return { + confirmed, + mutationFindings: mutation ? mutation.findings : [], + mutationSearched: mutation ? mutation.searched : '', + refuted: verified.filter(v => !v.survives).map(v => ({ title: v.title, file: v.file, line: v.line, lenses: v.lenses, reasons: v.verdicts.filter(x => x.refuted).map(x => x.reason), unverifiable: !!v.unverifiable })), + coverage, + gaps: gapList, + counts: { tasks: tasks.length, rawFindings: found.length, verified: verified.length, confirmed: confirmed.length }, +} diff --git a/.claude/skills/deep-audit/ledger.md b/.claude/skills/deep-audit/ledger.md new file mode 100644 index 0000000..b72b398 --- /dev/null +++ b/.claude/skills/deep-audit/ledger.md @@ -0,0 +1,8 @@ +# Deep-audit ledger + +Findings a `/deep-audit` run refuted, so the next run does not raise them again. A row holds only while +its reason is still true; once the code or the ruling behind it changes, the finding may come back with +the new evidence. Rows are appended by the skill and committed by the owner. + +| Range | Location | Claim | Refuted because | +|---|---|---|---| diff --git a/.claude/skills/loop-work/SKILL.md b/.claude/skills/loop-work/SKILL.md deleted file mode 100644 index 7a8d386..0000000 --- a/.claude/skills/loop-work/SKILL.md +++ /dev/null @@ -1,180 +0,0 @@ ---- -name: loop-work -description: The worker shared by both loops. Takes exactly one loop:ready issue, whether it came from /port-scan or /audit-scan, does the work in its own worktree and branch, fixes every site the issue lists rather than the one that reproduced, proves it with the browser-free suite, a live solve tally against a same-window baseline, and the indexer chain, then opens a draft PR carrying that evidence. It never merges, never pushes to main, and stops rather than guess. Use to land a queued item, or with --dry-run to see what it would do first. -argument-hint: "[--dry-run] [issue number] (omit to take the oldest loop:ready)" -disable-model-invocation: true -allowed-tools: - - Bash(git *) - - Bash(gh *) - - Bash(uv run *) - - Bash(docker *) - - Bash(curl *) - - Read - - Write - - Edit - - Glob - - Grep - - Skill ---- - -Land one queued issue end to end, and stop at a draft PR a person reviews. - -Both managers feed this one worker. `/port-scan` files `source:upstream` issues, `/audit-scan` files `source:audit` issues, and everything below is the same for both except where marked. The containment is the point: this skill can write code, so everything else about it is narrowed to **one issue per run**, one branch, one draft PR, and a hard stop the moment the work turns out to be bigger than the issue said. A person approves every merge. Nothing here merges, marks a PR ready, pushes to `main`, tags, or releases. - -## Arguments - -- `--dry-run` reads everything, mutates nothing, and prints the plan, the eligibility verdict, the exact commands, and the PR body it would write. It claims no issue, creates no branch, starts no container, and pushes nothing. Use it on the first run against any new issue shape. -- An issue number takes that issue. Omitted takes the oldest open `loop:ready`. - -## Step 1: Claim exactly one issue - -``` -gh issue list -R unseensnick/Solverr --label loop:ready --state open \ - --json number,title,createdAt,labels --limit 20 -``` - -Take the oldest, or the one named in `$ARGUMENTS`, and read its `source:` label to know which manager filed it. Then stop if any of these hold, and say which: - -- The queue is empty. That is a normal, successful, no-op run. -- The issue carries `loop:needs-human`. That label exists to stop this skill. Never take one, and never relabel one to `loop:ready` to get around it. -- The issue carries `loop:in-review`. It already has a PR. - -Claim it by removing `loop:ready` (`gh issue edit -R unseensnick/Solverr --remove-label loop:ready`). If the run bails later for any reason, put the label back unless the bail reason was an escalation to `loop:needs-human`. - -## Step 2: Preflight - -Refuse to start on a dirty tree. All of these must hold, and a failure is a stop, not something to work around: - -- `git status --porcelain` is empty. -- The current branch is `main` and it matches `origin/main` (`git fetch origin && git rev-parse main origin/main`). -- No worktree or branch already exists for this issue (`git worktree list`, `git branch --list 'loop/*'`). -- Docker is up, and the ports this run needs are free (`docker ps -a`). - -## Step 3: Worktree and branch - -``` -git worktree add .worktrees/loop- -b loop/- origin/main -``` - -`.worktrees/` is gitignored. The slug is a short kebab summary and, like every branch name here, carries no site names. Confirm the hooks reach the worktree (`git -C .worktrees/loop- config --get core.hooksPath` should read `.githooks`); local config is shared across worktrees, so this is a check rather than a step. Never bypass a hook with `--no-verify`. - -Every command from here runs with `-C .worktrees/loop-` or after a `cd` into it. The shell resets between calls, so assume neither. - -## Step 4: Scout first, and treat its verdict as a gate - -Run `/scout` against the issue before editing anything. This is the escalation gate, and it is the single most important step in the skill. - -**Stop and escalate** if scout reports any of the following: the change reaches the engines, sessions, or the controller in a way the issue did not anticipate; it collides with a "Deliberately different" ledger entry; the real scope is wider than the issue's checklist and the extra sites land in a tripwire zone; or it ends with a blocking open question. Escalating means: post the scout report as an issue comment, relabel `loop:needs-human`, remove the worktree and delete the branch, and report what stopped it. - -**Do not implement a smaller version of the task instead.** A narrowed fix that passes the gates is the worst outcome this loop can produce, because it looks like success. - -## Step 5: Implement, at every site - -Follow the scout plan. Then, before calling the code done, re-run the issue's own scope search and confirm every listed site is addressed. `.claude/rules/code-quality.md` is binding here: a bug present in five places is one bug with five sites, and the reported case passing is not the bug being fixed. Solverr's two engines were written against each other, so a defect in one usually has a twin in the other. - -Three outcomes are acceptable and no others. Fix every site. Or fix some and **list the rest in the PR body with the reason**, which is a decision the reviewer can see and overrule. Or escalate, if the full fix would reach a tripwire zone. - -Refactor when the correct fix needs it, in the same change, with the reason in the commit body. Extracting a helper, changing a signature, or moving a call site is the proper fix when the alternative is threading a workaround through the shape that is already there. What stays out of scope is adjacent cleanup that no part of this issue motivates. - -Alongside the code: - -1. **Tests**, if the change is reachable from the browser-free suite. It usually is. A fix at five sites gets coverage that would fail if any one of them regressed. -2. **`CHANGELOG.md`**, only when a deployer or an API client could notice. Behavior, config, response shape, image. A change with no observable effect gets a plain line under `Other`, and contributor tooling gets no entry at all. -3. **`docs/dev/upstream-sync.md`**, for `source:upstream` issues only: move the "Audited through" SHA for that upstream and add any new divergence with its reasoning. **This is the loop's state advance and it belongs in the branch**, so the ledger moves when the PR merges and not before. A port that leaves the ledger behind gets re-filed by the next scan. `source:audit` issues never touch the ledger. - -Commit with the message standard from `.claude/rules/workflow.md`. Check the message against the pre-commit checklist before running the command: the hook aborts a chained `git add && git commit`, so a bad message costs the whole chain. - -## Step 6: Verify, in three gates, cheapest first - -### Gate A, the browser-free suite - -``` -PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src -``` - -138 tests, no browser, seconds. A hard gate: any failure stops the run. Fix it or escalate, never delete or skip the test. - -### Gate B, the live solve tally - -Run `/live-check` scoped to what changed, and read its rules before starting: a single passing request proves nothing. - -Build and run on ports that leave the defaults alone: - -``` -docker build -t solverr:loop . -docker run -d --name solverr-loop --shm-size=512m -p 8291:8191 \ - -e LOG_LEVEL=debug -e STEALTH_ENGINE=true -e DEFAULT_ENGINE=chrome solverr:loop -``` - -**Build the baseline in the same window.** A second container off `origin/main` (`solverr:loop-base` on 8391), run trial for trial against the same testers, interleaved rather than one batch after the other. Cloudflare's behavior drifts by the hour, so a baseline measured yesterday is not a baseline. - -The verdict is a tally, never a single result: - -- **Within noise of the baseline**: pass. Record the numbers anyway. -- **Clearly worse than the baseline**, same testers, same window: fail. Comment the numbers on the issue, relabel `loop:needs-human`, keep the branch, and open no PR. -- **Ambiguous**: both arms low, or the failures look like refusals rather than breakage (several distinct `__cf_chl_tk` values inside one request, or the bare-browser probe failing too). Open the draft PR anyway, label it `needs-live-recheck`, and put the raw numbers and the reason in the body. An ambiguous window is a reason to ask a person, not to block or to bluff. - -Never report a live result as pass or fail without the trial count behind it. - -### Gate C, the consuming chain - -The chain is what actually uses Solverr, so it gets exercised. **There are two of them and they are different code paths. Pick the one the change is in, or the gate proves nothing.** - -- **Solverr's own passthrough** (`src/passthrough.py`, port 8888) when the change touches the passthrough or its config. Run the container with `PASSTHROUGH_ENABLED=true`, `PASSTHROUGH_ALLOWED_HOSTS` set, and drive it directly on 8988. `byparr-proxy` is **not** in this path: it fronts `/v1`, so putting it in front would exercise code the change never touches. -- **`byparr-proxy` in front of `/v1`** for everything else, since that is how the deployed stack runs. A fresh proxy built from `../byparr-proxy` on 8988, its `BYPARR` pointed at the run's own container (`http://solverr-loop:8191/v1`) over a private network. - -Either way, add a throwaway Prowlarr on 9796 when the change could affect what an indexer sees, import the definition from `../byparr-proxy/definitions/`, and run **one** search. Never 9696 or 8888, and never the owner's own Prowlarr: it gives a request about 100 seconds and then disables the indexer, so a slow solve reads as a broken indexer. On a throwaway instance that costs nothing. Report elapsed time next to the result count; a search that succeeds at 95 seconds is a finding. - -**When the change is a bound (a cache cap, a pool size, a limit), set it small enough that the boundary is reachable in a short run.** A production-sized default is never hit in five requests, so the gate would pass without touching the new code at all. Size it against a **measured** body rather than a guess: measure one real response first, then choose a bound that both admits it and is exceeded by a handful of them. Getting this wrong is not obvious, it just looks like a clean pass. - -## Step 7: Review - -Run `/pr-review` against the branch diff. Fix every High finding before opening the PR, or escalate if a fix would widen the change into a tripwire zone. Medium and Low findings go in the PR body for the human to weigh. - -## Step 8: Open the draft PR - -``` -git -C .worktrees/loop- push -u origin loop/- -gh pr create --draft -R unseensnick/Solverr --base main --head loop/- \ - --title "" --body-file -``` - -Draft, always. The body carries the evidence, in this order: - -1. **What changed and why**, plain language first. -2. **Provenance**: the upstream commit with its full SHA and link for `source:upstream`, or the audit finding and how it was verified for `source:audit`. -3. **Scope covered**: the issue's site checklist, each line marked fixed or deliberately left, with the reason for anything left. A reviewer must be able to see partial coverage without reading the diff. -4. **Gate A**: the test count and result. -5. **Gate B**: the tally table, new against baseline, trials per engine per tester, elapsed times, and the verdict with its reasoning. -6. **Gate C**: result count and elapsed time for the search. -7. **What was not covered.** An unverified path claimed as verified is worse than an admitted gap. -8. **Review findings** that were not fixed. -9. `Closes unseensnick/Solverr#`, in the explicit `owner/repo#N` form. A bare `#N` violates the message standard. - -Then relabel the issue `loop:in-review`, and add `needs-live-recheck` to the PR if gate B was ambiguous. - -## Step 9: Clean up, then report - -Remove everything this run created except the branch and the PR: both `-loop` containers and images, the throwaway Prowlarr and proxy, their network and volumes, and the worktree (`git worktree remove .worktrees/loop-`). The branch stays, because the PR points at it. - -Then report: the issue taken, the branch, the PR URL, the site checklist with its coverage, the three gates with their numbers, what was escalated, and confirmation that the cleanup left nothing behind (`docker ps -a`, `git worktree list`). - -## Never - -- Merge, mark a PR ready for review, approve, or close a PR. -- Push to `main`, force-push anything, tag, or cut a release. -- Take more than one issue in a run, or take a `loop:needs-human` issue. -- Relabel a `loop:needs-human` issue to `loop:ready`. -- Bypass a git hook, delete or skip a failing test, or silence a linter. -- Rewrite a command in PowerShell to get past a guard that stopped it in Bash. The hook matches both, and trying is itself a reason to stop and report. -- Edit anything under `../FlareSolverr`, `../Byparr`, or `../byparr-proxy`. -- Report a live gate as passing from one request. -- Put a site name in a branch name, commit, PR title, PR body, or CHANGELOG entry. - -## Rules - -- One run, one issue, one branch, one draft PR. -- Every site the issue lists is fixed, or listed as left with a reason. Silence is not an answer. -- Stop and escalate rather than shrink the task to fit what the loop can do alone. -- Every claim in the PR body is a number or a `file:line`, never an assurance. -- No em dashes. Commas, parentheses, periods, colons. diff --git a/.claude/skills/port-scan/SKILL.md b/.claude/skills/port-scan/SKILL.md deleted file mode 100644 index 9a0c4f0..0000000 --- a/.claude/skills/port-scan/SKILL.md +++ /dev/null @@ -1,140 +0,0 @@ ---- -name: port-scan -description: The manager half of the upstream port loop. Compares Byparr and FlareSolverr against the sync ledger, decides which new upstream commits are portable, and files one labeled GitHub issue per portable change so the worker loop has a queue. Triage only, it has no file-writing tools and never creates a branch. Use on a schedule, or when an upstream has moved and you want the work itemised before deciding what to act on. -argument-hint: "[--dry-run] [byparr|flaresolverr] (omit for both, --dry-run to file nothing)" -disable-model-invocation: true -allowed-tools: - - Bash(git *) - - Bash(gh issue *) - - Bash(gh label *) - - Read - - Glob - - Grep ---- - -Scan the upstreams for work that has not reached Solverr, and turn each portable change into one issue the worker loop can pick up. - -This skill triages. **It has no Edit or Write tool and that is deliberate**: a manager that can also write code stops being a check on the worker. It never creates a branch, never touches `src/`, and never advances the ledger. The ledger moves when a port merges, not when it is spotted. - -## Arguments - -Parse `$ARGUMENTS` before anything else: - -- `--dry-run` prints every issue it would file, in full, and creates nothing. **Always dry-run first on a ledger you have not scanned before.** -- `byparr` or `flaresolverr` narrows to one upstream. Omitted means both. - -## Step 1: Refresh the reference clones - -The siblings are read-only checkouts and they go stale. Fetch refs without touching either working tree, then read the remote branch rather than the local checkout: - -``` -git -C ../Byparr fetch --quiet origin -git -C ../FlareSolverr fetch --quiet origin -git -C ../Byparr log --oneline ..origin/main -git -C ../FlareSolverr log --oneline ..origin/master -``` - -Byparr's default branch is `main`, FlareSolverr's is `master`. A `fetch` is the only write either clone ever gets from this skill. Never check out, reset, pull, or edit anything under `../Byparr`, `../FlareSolverr`, or `../byparr-proxy`. - -If a range comes back empty for an upstream, say so and skip it. An empty Byparr range is the expected result most days, and an empty FlareSolverr range is the expected result most months. - -## Step 2: Read the state before judging anything - -Three sources, all of them binding: - -1. **`docs/dev/upstream-sync.md`**, the ledger. The "Audited through" table gives the range start. The "Deliberately different" list is the standing set of refusals: **an upstream commit that re-litigates one of those is not an issue**, it is already answered. The per-audit notes also record "not applicable" verdicts by SHA; do not re-file those either. -2. **Issues already filed**, open and closed, so a repeat scan is idempotent: - ``` - gh issue list -R unseensnick/Solverr --state all --label source:upstream --limit 100 --json number,title,state - ``` - Dedupe on `source:upstream`, the one label every issue from this skill carries, so the query catches them in any `loop:` state. **Do not list the `loop:` labels instead: repeated `--label` flags are ANDed, not ORed**, so asking for `loop:ready` and `loop:needs-human` and `loop:in-review` together matches nothing and every commit looks unfiled. Verified against this repo, where that form returned 0 while three matching issues existed. - - Every issue this skill files carries its upstream SHA in the title, which is what makes matching a commit reliable. A commit with an issue already open, or with a closed issue, is done. -3. **`Handoff.md`** if present, for recorded dead ends. Something listed under "What failed" is not eligible, whatever upstream did with it. - -## Step 3: Classify each commit - -Read the actual diff (`git -C ../Byparr show `), not the subject line. Subjects lie about scope, and Byparr's arc in the last audit was mostly commits that undid earlier commits. - -Every commit lands in exactly one bucket: - -- **Not applicable.** Upstream code with no counterpart here (the `/load` endpoint, their ruff config, their CI, test churn against files Solverr does not have). No issue. Record it in the scan summary so the next scan does not re-reason it. -- **Already covered.** Solverr does the same thing by another route. Cite both sides with `file:line`. No issue. -- **Deliberately refused.** It matches a "Deliberately different" ledger entry. No issue, cite the entry. -- **Portable.** Everything else. One issue. - -Then decide the label for each portable commit, and **default to `loop:needs-human` whenever the answer is not obvious.** Broad autonomy is where this loop would hurt. - -Eligibility is about **how well the change is understood, never about how few lines it touches.** A rule counting files would teach the worker to port the one call site that made the issue readable and leave its siblings alone, which is the failure `.claude/rules/code-quality.md` names directly: the reported case passing is not the bug being fixed. Solverr has two engines written against each other, so an upstream fix to one usually has a counterpart in the other. - -`loop:ready` requires all of: - -- **The full scope is enumerated in the issue.** Before labeling, search recursively for every place the change applies (both engines, the controller, the detection lists) and list each with `file:line`. Use `grep -rn '' src/ --include=*.py`, never `src/*.py`: a top-level glob skips `src/engines/` and `src/bottle_plugins/` and reports a confident zero. If the search cannot be made exhaustive, the scope is not known, so the label is `loop:needs-human`. -- It is covered by the browser-free suite, or the change is provably inert to solving (a log line, a message string, a bounds check). -- No ledger divergence entry names any file in that scope. -- It changes neither the `/v1` request or response shape nor the `"FlareSolverr is ready!"` banner. - -A change spanning four files with all four identified is a better `loop:ready` candidate than a one-file change whose blast radius nobody has checked. - -`loop:needs-human` if any of these are true, and say which one: - -- It touches the widget measuring or click path in `src/engines/stealth_engine.py`. -- It touches the shared `maxTimeout` budget split in `src/flaresolverr_service.py`. -- It touches the `quote()` calls in `src/postform.py`. That code has been "fixed" once already and the fix was wrong. -- It touches session lifecycle or the reaper (`src/sessions.py`, `src/session_reaper.py`). -- It touches `src/geo.py`, where the timezone and locale are paired on purpose. -- It moves a dependency pin, especially the stealth browser stack. Those get `/live-check` and a human before they merge, which is the whole reason the Renovate rule exists. -- It reopens a documented divergence, or the reasoning behind that divergence may no longer hold. -- You are not certain which bucket it belongs in. - -## Step 4: File the issues - -One issue per portable commit. Title carries the upstream and the short SHA so the dedupe in step 2 works: - -``` -port(byparr 2852dc2): report an unreachable target as a gateway failure -``` - -Body, in this order and nothing else: - -1. **Upstream commit**, full SHA and a link, plus the files it touches upstream. -2. **What it does**, two or three sentences, from reading the diff. -3. **Scope here**, as a checklist of every site the change applies to, each with `file:line`, and the search that produced the list so the worker can re-run it. This is the section that stops a partial fix from looking finished. -4. **Why it is portable**, and for `loop:needs-human`, which trigger from step 3 fired. -5. **Ledger context**: any entry that bears on it, quoted. -6. **Suggested verification**, which checks from the live matrix would prove it. - -Site names never appear in an issue title or body. `.claude/rules/workflow.md` covers commits, README, CHANGELOG and release notes; issues are a public surface too, so the same rule applies. Use "a Cloudflare-gated site" or "an indexer". - -Ensure the labels exist before filing (`gh label list -R unseensnick/Solverr`), and create any that are missing: - -Every issue this skill files also carries `source:upstream`, which is how `/loop-work` tells it apart from an audit finding. - -| Label | Meaning | -|---|---| -| `loop:ready` | Eligible for the worker loop to take unattended. | -| `loop:needs-human` | Real work, but the worker must not start it alone. | -| `source:upstream` | Filed here, from an upstream commit. | -| `loop:in-review` | The worker opened a draft PR. Set by the worker, never here. | -| `needs-live-recheck` | The live tally was ambiguous. Set by the worker, never here. | - -Always pass `-R unseensnick/Solverr` on `gh` calls. It is redundant now that the FlareSolverr remote is gone, and it costs nothing to keep the habit. - -## Step 5: Report - -A short summary to the chat, whatever the outcome: - -- The commit range scanned per upstream, and the head SHA each was scanned to. -- Counts per bucket, then the issues filed with their numbers and labels. -- Anything skipped as already-open, so a repeat run is visibly a no-op. - -**Do not update `docs/dev/upstream-sync.md`.** The ledger records what has landed. Moving it here would orphan every issue this scan just filed, and the next scan would see a clean range and file nothing. - -## Rules - -- No Edit, no Write, no branches, no PRs, no merges. Triage only. -- Never edit anything under `../FlareSolverr`, `../Byparr`, or `../byparr-proxy`. A `fetch` is the one exception. -- Read the diff before classifying. A subject line is not evidence. -- When the bucket or the label is genuinely unclear, `loop:needs-human` and say why. Guessing costs the owner a bad PR; escalating costs them one glance. -- A ledger divergence is cited, never re-argued. -- No em dashes. Commas, parentheses, periods, colons. diff --git a/.claude/skills/pr-review/SKILL.md b/.claude/skills/pr-review/SKILL.md index 2d337aa..a022043 100644 --- a/.claude/skills/pr-review/SKILL.md +++ b/.claude/skills/pr-review/SKILL.md @@ -14,7 +14,7 @@ Check `$ARGUMENTS` for the word `verbose`. Strip it from the argument string bef - **Default**: terse output. Each finding is one line (`file:line: issue (fix: hint)`). Synthesis report stays compact. - **`verbose`**: full breakdown. Each finding gets the multi-field block (Severity, Confidence, etc.). Synthesis report uses the full template. -When dispatching reviewers in Step 3, include the word `verbose` in each `Task` call's prompt only if the user asked for it. Otherwise omit; the reviewers default to terse. +When dispatching reviewers in Step 3, include the word `verbose` in each `Agent` call's prompt only if the user asked for it. Otherwise omit; the reviewers default to terse. ## Step 1: Determine Scope @@ -56,9 +56,9 @@ Decide which reviewers apply by reading the diff content, not just file paths: | `performance-reviewer` | Endpoints, DB queries, loops over collections, caching, connection management. Skip for pure-docs, config-only, or static-asset diffs. | | `doc-reviewer` | `.md` changes, significant docstring or JSDoc changes, API docs. | -**Dispatch all applicable reviewers in PARALLEL.** Send one message that contains one `Task` tool call per applicable reviewer (use `subagent_type` matching the reviewer name). Do NOT invoke them sequentially. Parallel dispatch cuts wall-clock time from N times the slowest review to roughly the slowest single review, with no extra token cost. +**Dispatch all applicable reviewers in PARALLEL.** Send one message that contains one `Agent` tool call per applicable reviewer (use `subagent_type` matching the reviewer name). Do NOT invoke them sequentially. Parallel dispatch cuts wall-clock time from N times the slowest review to roughly the slowest single review, with no extra token cost. -If only one reviewer applies (a pure-docs diff, for example), a single `Task` call is fine. Skip the parallel pattern when there's nothing to parallelize. +If only one reviewer applies (a pure-docs diff, for example), a single `Agent` call is fine. Skip the parallel pattern when there's nothing to parallelize. While the reviewers run, you can read the PR description, recent CI logs, or open comments to enrich the synthesis in Step 4. Don't wait idly. diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index e45f2ab..2e1e17d 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -31,7 +31,7 @@ Flag anything in the block that could surprise an existing client, and make sure Stop and report rather than continuing if any of these fail: 1. `git status` is clean apart from what you are about to commit, and the branch is `main`. -2. The browser-free tests pass: `PYTHONPATH=src uv run --no-project python -m unittest test_detection test_response_shape test_request_validation test_browser_identity test_geo`. +2. The whole browser-free suite passes, never a hand-picked module list: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src`. 3. Compile check: `uv run --no-project python -m py_compile src/*.py src/engines/*.py`. 4. `[Unreleased]` actually has entries. An empty block means there is nothing to release. 5. The version in `package.json` is the previous one, and no tag for the new version exists yet (`git tag -l v`). @@ -42,7 +42,7 @@ Stop and report rather than continuing if any of these fail: 1. Rename `## [Unreleased]` to `## []` and add a fresh empty `## [Unreleased]` above it. 2. Bump `version` in `package.json`. 3. Bump the `"version"` field in the `README.md` `/v1` response example. It is easy to miss because nothing fails without it: the docs simply keep advertising the previous release. Check with `grep -n '"version"' README.md`, which should return exactly one line and it should read the version being cut. This was missed on the 1.5.0 cut. -4. Commit as `chore(release): `, nothing else in that commit. +4. Commit as `chore(release): `, holding those three edits and nothing else. ## Step 4: Push, then tag @@ -70,6 +70,6 @@ Report the tag, the release URL, both workflow outcomes, and the digests. Then g - Never push a tag without explicit confirmation in the same conversation. - Never force-push, and never re-tag a published version. A mistake gets a new patch version. -- The release commit contains only the CHANGELOG rename and the version bump. +- The release commit contains only the CHANGELOG rename, the `package.json` version bump, and the version in the `README.md` response example. - Re-running `release.yml` by hand regenerates the notes and discards any manual edit to the release body. Say so if the body was edited. - No em dashes. Commas, parentheses, periods, colons. diff --git a/.claude/skills/scout/SKILL.md b/.claude/skills/scout/SKILL.md index 75cb0c4..f1952a2 100644 --- a/.claude/skills/scout/SKILL.md +++ b/.claude/skills/scout/SKILL.md @@ -52,7 +52,8 @@ Brief one `Agent` with `subagent_type: Explore` per area that applies, all in a | Area | When relevant | What to brief | |---|---|---| | Current Solverr code | Always | Target files, callers, callees, the controller path that reaches them, existing tests. | -| Upstream equivalent | Ports, drift checks | `../FlareSolverr` for the Chrome engine, `/v1`, sessions; `../Byparr` for the stealth stack. Hand over the matching file paths and `git -C log --oneline -15 -- `. Ask what differs and why. | +| Upstream equivalent | Ports, drift checks | `../FlareSolverr` for the Chrome engine, `/v1`, sessions; `../Byparr` for the stealth stack. Hand over the matching file paths and `git -C log --oneline -15 -- `. Ask what differs and why. When the upstream file has no Solverr counterpart of the same shape (FlareSolverr's `_evil_logic` and `sessions.py` were taken over), ask which spine site now carries that behaviour: the change lands there, not in a file rebuilt in upstream's shape. | +| Engine layer | Anything touching an engine, or any port | `.claude/rules/engine-layer.md`, its seam-depth table above all. Shared spine: `src/assembly.py`, `src/pipeline.py`, `src/budget.py`, `src/sessions.py`, `src/config.py`, and `src/dtos.py` for request validation. Per engine: each adapter in `src/engines/`, and each clearing core (Chrome's challenge wait in `_evil_logic` and its turnstile helpers; stealth's `_wait_until_cleared`, the widget measuring in `_widget_box` and the click in `_click_turnstile`). Ask which of those the change touches. A port of a taken-over surface goes into the spine, and a client-visible change lands for both engines in one commit. | | Engine and runtime constraints | Anything touching solving | `src/async_runtime.py` (one shared loop), `src/engines/stealth_engine.py` (per-context lock, throwaway click page), `src/session_reaper.py`. Ask what runs on which thread and what is serialized. | | Contract surface | Anything reaching a response | `src/dtos.py`, `_to_challenge_resolution` in `src/flaresolverr_service.py`, `utils.object_to_dict`. Ask what an unset field serializes to. | | Existing helpers (DRY) | New helper tempting | `src/detection.py`, `src/config.py`, `src/postform.py`, `src/utils.py`. Search before letting the plan invent one. | @@ -79,6 +80,7 @@ Additions specific to this skill: - **A claim without a `file:line` from code actually read** goes in Open questions, never in Findings. - **A source contradiction is itself a finding.** When memory, `Handoff.md`, or a doc disagrees with current code, trust the code and record it under Stale docs. +- **Deferred work is not a defect.** If the deferred or dead-end list from Step 2 already covers something, say so once and move on. - **The plan names the helper it reuses**, citing it. A step that invents a utility the repo already has is a failed scout. - **Say whether the change is verifiable without a browser.** If it can only be proven against a live challenge, the plan's last step is `/live-check`, not "run the tests". @@ -92,6 +94,7 @@ End with one of: **"Ready to implement."**, **"Open questions block implementati - Every claim cites `file:line` from current code. Memory and `Handoff.md` claims are hypotheses until cited. - Never fill an unresolved gap with an assumption. Investigate it or surface it. - Cap each subagent at ~500 words; the report follows the ~1500 word cap in `plan-output.md`. A bigger task needs decomposition, not a longer report. +- No interim narration: one sentence when the agents are spawned, then nothing until the report. - No em dashes. Commas, parentheses, periods, colons. -- No assumptions about library behavior. If the plan leans on what Playwright, Selenium, or playwright-captcha does, cite the installed source under `.venv/Lib/site-packages/`, not documentation from memory. +- No assumptions about library behavior. If the plan leans on what Playwright, Selenium, or playwright-captcha does, cite the installed source under `.venv/Lib/site-packages/`, not documentation from memory. Check the package's `.dist-info` version against `requirements.txt` first: the local `.venv` can lag the pin, and then the source it shows is not what ships. - If an Explore agent comes back vague, the brief was too loose. Re-spawn with a sharper scope before synthesizing. diff --git a/.claude/skills/session-handoff/SKILL.md b/.claude/skills/session-handoff/SKILL.md index 492b3d0..89d81ed 100644 --- a/.claude/skills/session-handoff/SKILL.md +++ b/.claude/skills/session-handoff/SKILL.md @@ -40,12 +40,15 @@ Never write the handoff from conversation memory alone. These are the facts the Keep these sections. Omit one only when it genuinely has nothing, rather than writing a placeholder. +- **Read first**: the two or three docs that frame the work, most authoritative first. For engine work that is `.claude/rules/engine-layer.md`, then `docs/dev/engine-layer-architecture.md`; for a port, `docs/dev/upstream-sync.md`. - **Goal**: the program-level outcome, not the next tactical step. What the fork is for, plus what this session was actually about. - **Current state**: what works, what is half-done, what is deliberately not done. Lead with the branch, HEAD SHA, pushed/unpushed count, and tree state. Include the release and container facts from Step 1. Name measured effects where there are any (timings, tallies, digests), because "it works" does not survive a week. - **Files**: only files central to the in-progress work, each with its role and status (new / modified / needs-attention). A handoff listing forty files is noise. Point at the browser-free test command when tests changed. - **Changes made**: a line per commit, grouped when a group tells the story better. Mark which are verified live and which are not. - **What failed**: the highest-value section. The approach, what was expected, what happened. Group variations of one idea so the next session does not try variation four. Include process failures (a wrong tool invocation, a bad assumption about the harness, a probe that tested the wrong thing) alongside code ones, and record corrections to your own earlier conclusions. - **Next steps**: ordered and concrete, saying which step gates which. Branch the ones that depend on an unknown. +- **Parked (do not raise unprompted)**: items blocked on an owner decision. Say plainly that the next session should not raise them unprompted, and where the detail lives (an issue as `owner/repo#N`, a ledger entry, a memory). +- **Durable gotchas**: facts that will outlive this session, such as a measured constraint, a deliberate exception, or a tool trap. Each one also goes to a memory or `CLAUDE.md` per the split in Step 3; the handoff copy is the reminder, not the record. - **Conventions**: the repo rules the next session must respect from its first action, including the naming rule and the `/v1` compatibility constraints. ## Step 3: The doc set @@ -68,7 +71,7 @@ For the memory pass, prefer updating an existing file over adding a near-duplica ## Step 4: Commit and verify 1. Commit tracked doc changes (`CHANGELOG.md`, `README.md`, `CLAUDE.md`, `docs/dev/upstream-sync.md`) with a `docs(...)` subject. `Handoff.md` is never in that commit. -2. Run the browser-free tests if any code changed: `PYTHONPATH=src uv run --no-project python -m unittest test_detection test_response_shape test_request_validation test_browser_identity test_geo`. +2. Run the whole browser-free suite if any code changed, never a hand-picked module list: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src`. 3. Report the final branch state, so the owner knows whether anything is left to push, and whether the tag and image are published or pending. ## Solverr conventions the global skill cannot know diff --git a/.claude/skills/upstream-audit/SKILL.md b/.claude/skills/upstream-audit/SKILL.md index ff28ac1..5b2a7fc 100644 --- a/.claude/skills/upstream-audit/SKILL.md +++ b/.claude/skills/upstream-audit/SKILL.md @@ -1,8 +1,8 @@ --- name: upstream-audit -description: Audit Solverr against its two upstreams for drift. Compares the stealth engine against Byparr and the Chrome engine, sessions and the /v1 surface against FlareSolverr, classifying every difference as already covered, genuinely missing, or deliberately divergent, and checking that the /v1 request and response shape is still byte-compatible. Use when either upstream has moved, before a release, when a fix lands upstream that might apply here, or when the owner asks whether Solverr has fallen behind. Ends by updating the sync ledger. -argument-hint: "[scope] (e.g. 'stealth engine', 'contract only', omit for the full sweep)" -disable-model-invocation: true +description: Audit Solverr against its two upstreams for drift. Compares each clearing core against its upstream file, walks the taken-over surfaces (assembly, pipeline, budget, sessions, config, dtos) as behaviour from upstream's side, checks dependency pins including transitive ones, and classifies every difference as already covered, genuinely missing, or deliberately divergent, then checks that the /v1 request and response shape is still byte-compatible. Use when either upstream has moved, before a release, when a fix lands upstream that might apply here, or when the owner asks whether Solverr has fallen behind. Reads only, and asks before its one write, the sync ledger. +argument-hint: "[scope] (e.g. 'stealth engine', 'contract only', 'pins', omit for the full sweep)" +disable-model-invocation: false allowed-tools: - Bash(git *) - Bash(uv run *) @@ -27,11 +27,15 @@ Cheap reads on the main thread, before spawning anything: One `Agent` per area, `subagent_type: Explore`, in a single message. Skip an area outside the requested scope. +The two kinds of surface are compared differently, per `.claude/rules/engine-layer.md`. A **clearing core** is still upstream-derived, so it is compared file against file. A **taken-over surface** (marked Done in that file's seam-depth table) no longer has upstream's shape, so a file diff says nothing useful about it: walk upstream's behaviour instead, and mark each item **present** (cite the spine site), **deliberately dropped** (cite the ledger), or **missing**. That rule's "A takeover is not complete until its behaviour is inventoried" is why this is a separate agent. + | Agent | Compares | Brief it to check | |---|---|---| -| Stealth vs Byparr | `src/engines/stealth_engine.py` vs `../Byparr/src/` | Browser launch options, the playwright-captcha framework and solver lifecycle, challenge detection lists, load-state waiting order, timeout handling, request options, dependency pins in `requirements.txt` vs `pyproject.toml`. | -| Chrome and sessions vs FlareSolverr | `src/engines/chrome_engine.py`, `src/sessions.py`, `src/utils.py` vs the same files upstream | Fixes upstream has that Solverr lacks, and files that are byte-identical (those need no review, say so). | +| Stealth clearing core vs Byparr | `_wait_until_cleared`, `_challenge_stays_gone`, `_click_turnstile`, `_widget_box`, `_frame_box`, `_container_box`, `_turnstile_token` in `src/engines/stealth_engine.py` vs `../Byparr/src/challenge.py` and `_navigate_and_solve` in `../Byparr/src/endpoints.py` | The algorithm and the widget constants by name and role, the confirm-after-clear re-check, the click cooldown, load-state waiting order. Also the adapter's browser launch options (`StealthContext.start`) vs `get_browser` in `../Byparr/src/utils.py`. | +| Chrome clearing core and inherited files vs FlareSolverr | The challenge wait in `ChromeEngine._evil_logic`, `click_verify`, `_get_turnstile_token`, `_resolve_turnstile_captcha` in `src/engines/chrome_engine.py` vs the same functions in `../FlareSolverr/src/flaresolverr_service.py`; `src/utils.py` and the ledger's byte-identical list vs the same files upstream | Fixes upstream has that Solverr lacks, and files that are byte-identical (those need no review, say so). Diff with `--strip-trailing-cr`: Solverr's copies are CRLF. | +| Taken-over surfaces, as behaviour | `src/assembly.py`, `src/pipeline.py`, `src/budget.py`, `src/sessions.py`, `src/config.py`, `src/dtos.py` vs what they replaced: the rest of FlareSolverr's `_evil_logic` (media blocking, the navigate-cookies-reload order, access-denied, `waitInSeconds`, `returnOnlyCookies`, the screenshot, result assembly), `SessionsStorage` in `../FlareSolverr/src/sessions.py`, upstream's `dtos.py` and `utils.get_config_*`, and Byparr's `read_item` in `../Byparr/src/endpoints.py` with `../Byparr/src/models.py` | Walk each upstream behaviour end to end, starting from upstream's code, never from ours, and mark it present, deliberately dropped, or missing. Include what upstream changed there since the ledger's audited-through commit. | | Contract vs FlareSolverr | `src/dtos.py`, `src/flaresolverr_service.py`, `src/flaresolverr.py` | Field names, types, which fields are emitted unset, error shape, status codes, command handling, the banner. | +| Dependency pins | `requirements.txt` vs `../Byparr/pyproject.toml` and `../Byparr/uv.lock`, and vs `../FlareSolverr/requirements.txt` | Direct pins, and the transitive versions Byparr's lock resolves under an unchanged floor: `invisible-playwright` carries `invisible-core`, which carries the patched Firefox, so a lock that moves either is drift even when `pyproject.toml` did not change. Compare against what Solverr's pin resolves to, recorded in the ledger's Taken section, not against the local `.venv`, which can lag the pin. | Demand `file:line` on both sides of every claimed difference. Cap each at ~500 words. Ask each for an explicit "identical, nothing to report" list, which is as useful as the differences. @@ -43,6 +47,8 @@ Every reported difference gets one label, and the label is the work: - **Genuinely missing.** Upstream has something Solverr does not, and it applies here. Cite it, say what it would take, and rate the impact. - **Deliberately different.** Solverr diverges on purpose. Cite the ledger entry or the code comment that records why. If nothing records it, that is itself a finding: the reason exists only in someone's head. +The behaviour inventory maps onto the same three labels: present is already covered, deliberately dropped is deliberately different, and missing is genuinely missing. + Then re-read, yourself, every difference you are about to call missing. The recurring false positive is an upstream fix Solverr already has under a different name, in a different layer, or with a different spelling. Solverr is ahead of both upstreams in places. Say so explicitly when you find it, because "do not port backwards" is a real failure mode. @@ -55,11 +61,13 @@ Follow [.claude/rules/plan-output.md](../../rules/plan-output.md). Three section 2. **Drift against FlareSolverr**, same. 3. **`/v1` contract compatibility**, which is pass or fail rather than graded: name any field renamed, dropped, retyped, or newly always-emitted, and confirm the banner is intact. +Takeover inventory items and pin drift go under the upstream they came from. + An audit with no implementation to propose omits the plan section. ## Step 5: Update the ledger -Whatever the outcome, update `docs/dev/upstream-sync.md`: the commit each upstream was audited through, today's date, and any new deliberate divergence with its reasoning. An audit that does not move the ledger forward will be run from scratch next time. +Whatever the outcome, propose the update to `docs/dev/upstream-sync.md`: the commit each upstream was audited through, today's date, and any new deliberate divergence with its reasoning. Write it once the owner confirms; it is the only file this skill writes. An audit that does not move the ledger forward will be run from scratch next time. If the audit found work, ask which items to act on. Do not start fixing during the audit: a fix mid-audit contaminates the rest of the comparison. diff --git a/.dockerignore b/.dockerignore index 4eaa27c..4c76b55 100644 --- a/.dockerignore +++ b/.dockerignore @@ -19,6 +19,5 @@ docs/ html_samples/ CHANGELOG.md CLAUDE.md -CLAUDE.local.md.example Handoff.md README.md diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..20d668b --- /dev/null +++ b/.editorconfig @@ -0,0 +1,15 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 2 +indent_style = space +insert_final_newline = true +trim_trailing_whitespace = true + +[*.py] +indent_size = 4 + +[*.md] +trim_trailing_whitespace = false diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..3aaf683 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,14 @@ +# Text is stored and checked out with LF on every platform. The git hooks and the +# .claude/hooks scripts run under bash, which breaks on CRLF, and both upstreams are LF, so a +# CRLF working copy made every line differ when diffing against them. +* text=auto eol=lf + +*.png binary +*.jpg binary +*.gif binary +*.ico binary +*.webp binary +*.pyc binary +*.gz binary +*.zip binary +*.mmdb binary diff --git a/.githooks/commit-msg b/.githooks/commit-msg index fca42b2..db966b3 100644 --- a/.githooks/commit-msg +++ b/.githooks/commit-msg @@ -22,9 +22,10 @@ echo "$subject" | grep -qE '^(feat|fix|docs|chore|refactor|test|perf|build|ci|st # 3) No em dash anywhere in the message echo "$msg" | grep -q '—' && errors+=("contains an em dash; use commas, parentheses, periods, or colons") -# 4) No AI watermark -echo "$msg" | grep -qiE 'co-authored-by|generated with|claude\.ai/code|🤖' \ - && errors+=("contains an AI watermark (Co-Authored-By / Generated with / robot emoji)") +# 4) No AI watermark. A Co-authored-by trailer naming a person is credit and passes; one +# naming an AI tool is a watermark. +echo "$msg" | grep -qiE 'co-authored-by:.*(claude|anthropic|openai|chatgpt|copilot|gemini|cursoragent)|generated with|claude\.ai/code|🤖' \ + && errors+=("contains an AI watermark (an AI Co-authored-by trailer / Generated with / robot emoji)") # 5) No BARE '#' reference: it links to an issue in THIS repo, which is rarely what was meant. # Strip explicit owner/repo#N first (FlareSolverr/FlareSolverr#1626), then flag what remains. diff --git a/.githooks/pre-commit b/.githooks/pre-commit index cf5df43..81dc0ee 100644 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Solverr docs enforcement (see .claude/rules/workflow.md: "CHANGELOG" and "Public-facing naming"). -# Lints staged CHANGELOG.md and README.md: +# Lints staged CHANGELOG.md, README.md, CONTRIBUTING.md and CLAUDE.md: # both - no target-site names or scraping vocabulary in newly added lines, no em dash. # CHANGELOG - every new [Unreleased] entry under Additions / Changes / Fixes leads with a # self-contained bold headline ending in . ! or ? (Other is exempt). @@ -29,7 +29,7 @@ name_patterns='(^|[^A-Za-z0-9./@-])[a-z0-9][a-z0-9-]{1,}\.(to|st|se|nl|cc|ws|eu| local_deny="$(dirname "$0")/deny-names.local" [ -f "$local_deny" ] && name_patterns="$name_patterns|$(paste -sd'|' "$local_deny")" -for f in CHANGELOG.md README.md; do +for f in CHANGELOG.md README.md CONTRIBUTING.md CLAUDE.md; do staged "$f" || continue new=$(added "$f") [ -n "$new" ] || continue diff --git a/.githooks/tests/run.sh b/.githooks/tests/run.sh new file mode 100644 index 0000000..11692b9 --- /dev/null +++ b/.githooks/tests/run.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# Checks that the git hooks reject what they claim to reject, and pass what they must pass. +# +# Both hooks are regexes, and a regex that silently stops matching looks exactly like a clean +# commit. Every rule below therefore has a case that must fail as well as one that must pass. +# CI runs this before it runs the hooks themselves (.github/workflows/standards.yml). +# +# Run it after touching either hook: bash .githooks/tests/run.sh +set -u + +hooks="$(cd "$(dirname "$0")/.." && pwd)" +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +pass=0 +broke=0 +em=$(printf '\xe2\x80\x94') + +# check +check() { + local name="$1" want="$2" + shift 2 + local got=0 + "$@" > /dev/null 2>&1 || got=1 + if [ "$got" = "$want" ]; then + echo " ok $name" + pass=$((pass + 1)) + else + echo " BROKE $name (wanted exit $want, got $got)" + broke=$((broke + 1)) + fi +} + +# msg +msg() { + printf '%b' "$3" > "$work/msg" + check "$2" "$1" bash "$hooks/commit-msg" "$work/msg" +} + +echo "commit-msg" +msg 0 "passes a conventional subject" 'fix(stealth): accept a cookie without a domain\n' +msg 0 "passes a subject with no scope" 'docs: explain the passthrough timeout\n' +msg 1 "rejects a subject with no type" 'Fixed the cookie bug.\n' +msg 1 "rejects an unknown type" 'update(api): tweak things\n' +msg 1 "rejects a subject over 72 characters" 'fix(api): this subject keeps going well past the seventy-two character limit\n' +msg 1 "rejects an em dash in the body" "fix(api): accept headers\n\nIt was refused ${em} now it is not.\n" +msg 1 "rejects an AI co-author trailer" 'fix(api): accept headers\n\nCo-Authored-By: Claude \n' +msg 1 "rejects a Copilot co-author trailer" 'fix(api): accept headers\n\nCo-authored-by: Copilot \n' +msg 1 "rejects a generated-with footer" 'fix(api): accept headers\n\nGenerated with a tool\n' +msg 0 "passes a human co-author trailer" 'fix(api): accept headers\n\nCo-authored-by: Jane Doe \n' +msg 1 "rejects a bare issue number" 'fix(api): accept headers\n\nSee #12.\n' +msg 0 "passes the owner/repo issue form" 'fix(chrome): port the focus fix\n\nFrom FlareSolverr/FlareSolverr#1626.\n' +msg 1 "rejects a squash-merge (#N) suffix" 'fix(api): accept headers (#11)\n' +msg 1 "rejects a domain-shaped site name" 'fix: clears example.to again\n' +msg 1 "rejects scraping vocabulary" 'feat: add a scraper mode\n' +msg 0 "lets a merge commit through" 'Merge pull request #11 from someone/branch\n\nfix(api): accept headers\n' + +# The pre-commit hook reads the index (or a commit range), so each case runs in a scratch repo. +repo="$work/repo" +g() { git -C "$repo" -c core.hooksPath=/nonexistent -c user.name=t -c user.email=t@example.com -c core.autocrlf=false "$@"; } +reset_repo() { + rm -rf "$repo" + mkdir -p "$repo" + g init -q + printf '# Changelog\n\n## [Unreleased]\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md" + printf '# Readme\n' > "$repo/README.md" + printf '# Contributing\n' > "$repo/CONTRIBUTING.md" + g add -A + g commit -q -m "chore: baseline" +} +# stage +stage() { + printf '%b' "$2" >> "$repo/$1" + g add "$1" +} +pre() { (cd "$repo" && bash "$hooks/pre-commit" "$@"); } + +echo "pre-commit" +reset_repo; stage README.md 'A plain new line.\n' +check "passes a clean README line" 0 pre +reset_repo; stage README.md "A line ${em} with an em dash.\n" +check "rejects an em dash in the README" 1 pre +reset_repo; stage CONTRIBUTING.md "A line ${em} with an em dash.\n" +check "rejects an em dash in CONTRIBUTING" 1 pre +reset_repo; stage README.md 'Point it at example.to for testing.\n' +check "rejects a site name in the README" 1 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Fixes\n\n- **A real headline.** Detail.\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "passes a bold CHANGELOG headline" 0 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Fixes\n\n- No bold headline here.\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "rejects an entry with no headline" 1 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Fixes\n\n- **A headline with no stop**\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "rejects a headline with no full stop" 1 pre +reset_repo +printf '# Changelog\n\n## [Unreleased]\n\n### Other\n\n- Bumped a dependency.\n\n## [1.0.0]\n' > "$repo/CHANGELOG.md"; g add CHANGELOG.md +check "exempts the Other section" 0 pre +reset_repo; stage README.md "A line ${em} in a commit.\n"; g commit -q -m "docs: add a line" +check "range mode rejects a committed em dash" 1 pre HEAD~1..HEAD + +echo "" +echo "$pass passed, $broke broke" +[ "$broke" -eq 0 ] diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..b845cbc --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,18 @@ +## Summary + + + +## How it was tested + + + +## Checklist + +See [CONTRIBUTING.md](../CONTRIBUTING.md) for what each item means. + +- [ ] The browser-free suite passes: `PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src` +- [ ] Commit messages follow `type(scope): summary`, at most 72 characters, with no em dash, no bare `#N`, and no AI attribution +- [ ] No names of target sites in commits, docs, or code comments (see CONTRIBUTING.md for the words the check also rejects) +- [ ] A change a client can notice lands for both engines, or the pull request names the capability one engine does not have +- [ ] The `/v1` request and response shape is unchanged, apart from new optional fields +- [ ] `CHANGELOG.md` has an `[Unreleased]` entry if someone running Solverr could notice the change, and none otherwise diff --git a/.github/workflows/release-docker.yml b/.github/workflows/release-docker.yml index ec48f36..223f21b 100644 --- a/.github/workflows/release-docker.yml +++ b/.github/workflows/release-docker.yml @@ -21,11 +21,11 @@ jobs: contents: read packages: write steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Docker metadata id: meta - uses: docker/metadata-action@v6 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: # metadata-action lowercases the image name (ghcr requires lowercase). images: ghcr.io/${{ github.repository }} @@ -36,18 +36,18 @@ jobs: # Move :latest only when building a version tag. flavor: latest=${{ github.ref_type == 'tag' }} - - uses: docker/setup-qemu-action@v4 - - uses: docker/setup-buildx-action@v4 + - uses: docker/setup-qemu-action@1f40c72289eff860ee54a304f1438e3cff362e0a # v4.3.0 + - uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0 - name: Log in to ghcr.io - uses: docker/login-action@v4 + uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: registry: ghcr.io username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - name: Build and push - uses: docker/build-push-action@v7 + uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 with: context: . # arm64 is built under QEMU emulation and is slow for this large diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5de08df..3324206 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -33,7 +33,7 @@ jobs: name: Create release runs-on: ubuntu-latest steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -47,7 +47,7 @@ jobs: fi - name: Install parse-changelog - uses: taiki-e/install-action@parse-changelog + uses: taiki-e/install-action@ffe3fd350e607ed6df4dd1765629bcc24853038a # parse-changelog - name: Prepare release body env: @@ -79,7 +79,7 @@ jobs: } > release_body.md - name: Create release - uses: softprops/action-gh-release@v3 + uses: softprops/action-gh-release@efb35369e0ad2afab669f228072c1b0d510eae64 # v3.0.3 with: tag_name: ${{ steps.vars.outputs.tag }} name: Solverr ${{ steps.vars.outputs.tag }} diff --git a/.github/workflows/standards.yml b/.github/workflows/standards.yml index c302942..08fa956 100644 --- a/.github/workflows/standards.yml +++ b/.github/workflows/standards.yml @@ -18,7 +18,7 @@ jobs: name: Commit messages and docs runs-on: ubuntu-latest steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 @@ -49,6 +49,15 @@ jobs: echo "head=$head" >> "$GITHUB_OUTPUT" echo "Checking ${base}..${head}" + # The hooks are regexes, and one that silently stops matching looks exactly like a clean + # commit, so prove they still reject what they should before trusting a pass from them. + - name: Hook self-test + run: bash .githooks/tests/run.sh + + # The Claude Code guards are regexes too, and they fail the same silent way. + - name: Claude Code hook guards + run: bash .claude/hooks/tests/run-all.sh + - name: Commit messages env: BASE: ${{ steps.range.outputs.base }} diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml new file mode 100644 index 0000000..d9ca6f2 --- /dev/null +++ b/.github/workflows/tests.yml @@ -0,0 +1,32 @@ +# Runs the browser-free suite on every pull request and every push to main. It needs no browser +# and takes seconds. Whether a page still clears a real challenge is outside what it can see; that +# is the live check's job before a release. +name: Tests + +on: + push: + branches: [main] + pull_request: + workflow_dispatch: + +permissions: + contents: read + +jobs: + tests: + name: Browser-free suite + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - uses: astral-sh/setup-uv@bec219d24cd3e171d82865faccec33120bb574f4 # v10.1.0 + with: + python-version: "3.14" + + - name: Install dependencies + run: | + uv venv + uv pip install -r requirements.txt -r test-requirements.txt + + - name: Run the suite + run: PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src diff --git a/CLAUDE.local.md.example b/CLAUDE.local.md.example deleted file mode 100644 index f3b1f53..0000000 --- a/CLAUDE.local.md.example +++ /dev/null @@ -1,26 +0,0 @@ -# Personal Overrides - -> Rename this to CLAUDE.local.md. It's gitignored and won't be shared with the team. - -## My Preferences - -- I prefer verbose commit messages with context -- Always explain your reasoning before making changes -- When in doubt, ask rather than guess - -## Environment - -- My test database runs on port 5433 (not default 5432) -- Use `pnpm` instead of `npm` on my machine - -## Shortcuts - -- When I say "ship it", run `/ship` -- When I say "review", run `/pr-review` -- When I say "fix it", run `/debug-fix` - -## Current Context - -- I'm working on the billing module this sprint -- The staging environment is at https://staging.example.com -- Feature flags are managed in LaunchDarkly diff --git a/CLAUDE.md b/CLAUDE.md index 7871a63..c91b3a8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,58 +7,61 @@ FlareSolverr fork with two solving engines and automatic fallback. Cloudflare/DD ```bash docker compose up -d --build # build + run (image bundles both browsers, ~2.3 GB) docker logs -f solverr # logs (set LOG_LEVEL=debug for more) +PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src # browser-free suite, seconds; CI runs it +bash .githooks/tests/run.sh # the git hooks still reject what they claim to +bash .claude/hooks/tests/run-all.sh # the Claude Code guard hooks, against their fixtures uv run --no-project python -m py_compile src/*.py src/engines/*.py # quick compile check -uv run python -m unittest src.tests # test suite (unittest + webtest; needs a browser) ``` -## Architecture (non-obvious) +Python only through uv; there is no system Python. `src/tests.py` is upstream's suite: it needs a real browser and live sites. Whether a page still clears a real challenge is `/live-check`, never the unit tests. -- Two engines behind one interface (`engines/base.py`): `chrome` (Selenium + vendored undetected_chromedriver, the default) and `stealth` (Camoufox via invisible_playwright + playwright-captcha). The controller auto-falls-back between them and remembers per-host which one cleared it. -- The stealth engine is async Playwright running on ONE background event-loop thread (`async_runtime.py`); persistent Camoufox contexts (sessions) live there so their cookies survive across requests. The server itself is synchronous. -- Sessions: each engine keeps its own pool, both using one `SessionStore` (`sessions.py`) so the lifecycle rules exist once; a background reaper (`session_reaper.py`) closes idle browsers. A session handed out is marked in use under the same lock that found it, which is what keeps the reaper and the cap off a live browser. Solve once, reuse the cookie many times. -- Escalation ladder for an `auto` request: Chrome → Camoufox click-solve → (optional, dormant) paid CAPTCHA API. -- **A Turnstile checkbox is clicked by coordinate, with no JS evaluation.** The widget's iframe sits in a closed shadow root, so `query_selector` cannot find it, but `page.frames` lists it anyway; `frame_element().bounding_box()` gives its rect and `page.mouse` clicks the checkbox. This exists because playwright-captcha's shadow-root traversal uses `evaluate_handle`, and the iframe's CSP blocks eval under Firefox, which silently broke widget solving. Cloudflare's own interstitial builds the widget itself and its frame reports an empty URL, so when no frame matches, the rect comes from the nearest ancestor `div` of the token input instead, which is in the light DOM. Do not reach for `page.evaluate` to measure any of this: running page scripts against a live challenge makes Cloudflare reissue it. -- **A challenge is only over once a clear reading survives a second look.** Cloudflare drops the challenge markup while it issues the next round, so believing the first clear reading returns an intermediate challenge page. -- **`maxTimeout` is one budget for the whole request, split evenly across the planned engines.** It used to be handed to each engine in full, so a fallback could take twice as long as asked and trip the caller's own timeout. An even share is what makes the fallback reachable: giving the first engine everything let it spend the lot, and a request that used to succeed in 133s failed at 120s with the second engine skipped. A quick first engine costs the fallback nothing, since the fallback inherits everything unspent. -- **The POST form is carried to the browser as a `data:text/html,` URL, so its fields are percent-encoded and must stay that way** (`postform.py`). The browser URL-decodes the document before the HTML parser sees it, so a value holding a bare `%` or `#` is otherwise re-read as an escape or truncates the document at the fragment. The `quote()` calls look like double-encoding and are not: removing them breaks POST for those values, measured against a live echo service. -- **Solverr resolves the browser's timezone and language itself (`geo.py`), and hands both engines the same pair.** Left alone, the stealth stack resolves both from the exit IP on every launch, inside the library, uncached, and raises behind a proxy when the lookup fails, which kills the launch; Chrome derived neither, so the two engines disagreed about the country. Passing concrete values returns before that fatal branch. They travel together because the pairing is what a site checks. Chrome follows via `Emulation.setTimezoneOverride` (which moves its ICU clock rather than patching `Intl` in the page) and `--accept-lang`. A failed lookup falls back to `TZ` and `en-US`: a wrong zone still solves, no browser does not. -- **playwright-captcha only ever touches a throwaway page**, and only the paid escalation reaches it now. Preparing a solver injects init scripts (one rewrites `Element.prototype.attachShadow`) that a Cloudflare interstitial will not clear while they are present, and Playwright cannot remove an init script. Verified live: an interstitial clears in ~3s without them and never in 40s with them. +## Working approach + +- **Memory and `Handoff.md` are hypotheses, not facts.** A memory that names a function, file or flag is true only if it still exists in current code. When one turns out stale, surface it for pruning instead of acting on it. +- **Plan steps carry their check inline**, as `1. -> verify: `, so a step nothing can check is visible before it is built. +- **Reply length.** Default replies are a few sentences: the answer or outcome, the detail that matters, done. A full report is for when the owner asks for one, or for a `/scout` or `/code-research` deliverable, which has its own cap in [.claude/rules/plan-output.md](.claude/rules/plan-output.md). + +## Architecture in brief + +Two engines behind one interface: `chrome` (Selenium + vendored undetected_chromedriver, the default) and `stealth` (Camoufox via invisible_playwright, on one background event-loop thread). The controller falls back between them and remembers per host which one cleared it. What both engines must do the same way lives once in the shared spine (`assembly.py`, `pipeline.py`, `budget.py`, `sessions.py`); each engine is an adapter over a clearing core derived from its upstream. Several constraints look wrong until you know what they were measured against (the coordinate Turnstile click, no `page.evaluate` against a challenge page, the second look before a challenge counts as cleared, the even `maxTimeout` split, the `quote()` calls in `postform.py`): read [.claude/rules/architecture.md](.claude/rules/architecture.md) before touching any of them. It loads on its own when you work in `src/`. ## Key decisions (WHY) -- **Fork on FlareSolverr, not Byparr.** FlareSolverr's Chrome engine already clears the target sites and has sessions; Python 3.11 + a vendored undetected_chromedriver let the Camoufox/Playwright stack coexist. Byparr pins Python 3.14, too new for undetected_chromedriver. +- **Fork on FlareSolverr, not Byparr.** FlareSolverr's Chrome engine already clears the target sites and has sessions, and its vendored undetected_chromedriver lets the Camoufox/Playwright stack run beside it in one Python 3.14 image. - **Reliability is dominated by IP reputation, not the tool.** A residential proxy (`PROXY_URL`) is the biggest lever; warm-session cookie reuse is the second. - **The consuming client keeps one shared session and never destroys it**, so the server-side reaper is what prevents leaked browsers (especially the heavier Camoufox ones). ## Where things live -- `src/flaresolverr.py` — entrypoint: logging setup (note the `force=True`), server, reaper start. -- `src/flaresolverr_service.py` — controller: `/v1` commands, engine selection + fallback, per-host memory, session commands. -- `src/assembly.py`, `src/pipeline.py`, `src/budget.py` — the shared spine: what a response contains and in what order, the page verdict and the navigate-cookies-reload order, the solve deadline. The first two are sans-io generators (they yield what to read, the engine supplies how) because one engine is synchronous and the other asynchronous; see `.claude/rules/engine-layer.md` before reshaping them. -- `src/engines/` — `base.py` (Engine + SolveResult), `chrome_engine.py`, `stealth_engine.py`. Each is an adapter over an upstream-derived clearing core, which is the one thing the spine never takes over. -- `src/async_runtime.py`, `src/session_reaper.py`, `src/sessions.py` — stealth event loop, idle reaper, and the `SessionStore` both engines use (each holds its own instance; the lifecycle rules live once). -- `src/detection.py` (shared challenge/title/selector lists), `src/geo.py` (browser timezone for both engines), `src/config.py` (env, including `env_proxy`), `src/postform.py`, `src/dtos.py` (request DTOs plus the type validation that makes their annotations binding). -- `src/engine_fakes.py` — drives either engine browser-free from one neutral `World`, for `test_engine_conformance.py`. Imported, not collected. -- `.claude/rules/engine-layer.md` — **the law for anything touching an engine**: write-once and its one exit, which code is upstream's and which is ours, capability slots, the pin-once ladder, and how deep the seam goes per surface. Loads every session. -- `.claude/rules/workflow.md` — CHANGELOG + commit rules, release-cut, public-facing naming, git hooks. `code-quality.md` — coding principles. `security.md` / `error-handling.md` — path-scoped to `src/`. `plan-output.md` — how a findings report or plan is structured. `prose-style.md` — sentence-level writing for every output. -- `docs/dev/engine-layer-architecture.md` — the rationale behind that law: the divergence measurements against both upstreams, the target seam, the sequencing, and every ruling with the evidence it rests on. Read it before designing anything forward-looking. -- `docs/dev/upstream-sync.md` — what has been taken from FlareSolverr and Byparr, through which commit, and every deliberate divergence with its reasoning. Read it before calling something drift. -- `docs/dev/loops.md` — the port loop's contract: what the manager and worker each own, what they may not do, the three verification gates, and the eligibility rules that keep the worker away from the engines. -- `.githooks/` — tracked commit-msg and pre-commit hooks. Activate with `git config core.hooksPath .githooks`. +- `src/flaresolverr.py`: entrypoint. Logging setup (note the `force=True`), server, reaper start. +- `src/flaresolverr_service.py`: controller. `/v1` commands, engine selection and fallback, per-host memory, session commands. +- `src/assembly.py`, `src/pipeline.py`, `src/budget.py`: the shared spine. What a response contains and in what order, the page verdict and the navigate-cookies-reload order, the solve deadline. The first two are sans-io generators (they yield what to read, the engine supplies how) because one engine is synchronous and the other asynchronous; see `.claude/rules/engine-layer.md` before reshaping them. +- `src/engines/`: `base.py` (Engine + SolveResult), `chrome_engine.py`, `stealth_engine.py`. Each is an adapter over an upstream-derived clearing core, which is the one thing the spine never takes over. +- `src/async_runtime.py`, `src/session_reaper.py`, `src/sessions.py`: stealth event loop, idle reaper, and the `SessionStore` both engines use (each holds its own instance; the lifecycle rules live once). +- `src/detection.py` (shared challenge/title/selector lists), `src/geo.py` (browser timezone and language for both engines), `src/config.py` (env, including `env_proxy`), `src/postform.py`, `src/dtos.py` (request DTOs plus the type validation that makes their annotations binding). +- `src/engine_fakes.py`: drives either engine browser-free from one neutral `World`, for `test_engine_conformance.py`. Imported, not collected. +- `.claude/rules/engine-layer.md`: **the law for anything touching an engine**. Write-once and its one exit, which code is upstream's and which is ours, capability slots, the pin-once ladder, and how deep the seam goes per surface. Loads every session. +- `.claude/rules/architecture.md`: the non-obvious architecture and its measured constraints, path-scoped to `src/`. +- `.claude/rules/workflow.md`: CHANGELOG and commit rules, merging, release-cut, public-facing naming, the git hooks and every check they run. `code-quality.md`: coding principles. `testing.md`: test rules and commands. `security.md` / `error-handling.md`: path-scoped to `src/`. `plan-output.md`: how a findings report or plan is structured. `prose-style.md`: sentence-level writing for every output. +- `CONTRIBUTING.md` and `.github/pull_request_template.md`: the same standard, written for outside contributors. Keep them in step with `workflow.md`. +- `docs/dev/engine-layer-architecture.md`: the rationale behind the law. The divergence measurements against both upstreams, the target seam, the sequencing, and every ruling with the evidence it rests on. Read it before designing anything forward-looking. +- `docs/dev/upstream-sync.md`: what has been taken from FlareSolverr and Byparr, through which commit, and every deliberate divergence with its reasoning. Read it before calling something drift. +- `.githooks/`: tracked `commit-msg` and `pre-commit` hooks, plus `tests/run.sh`, which proves each rule still rejects a real violation. Activate with `git config core.hooksPath .githooks`. CI runs the same checks (the Standards and Tests workflows). +- `.claude/hooks/`: the guards that screen tool calls before they run, so an unexplained `Blocked:` message comes from here. `block-dangerous-commands.sh` covers **both Bash and PowerShell** (matching only one lets a command through the other tool) and refuses a push to `main`, a bare force push (`--force-with-lease` is allowed), merging a PR (`gh pr merge` or through `gh api`), reading secret files through the shell, and the usual destructive deletes. Merging is always the owner's call. +- `.claude/agents/`: the four review subagents to spawn with the `Agent` tool: `code-reviewer`, `doc-reviewer`, `performance-reviewer`, `security-reviewer`. `/pr-review` runs all four in parallel. ## Skills -- `/scout` — investigate one non-trivial task, then produce its plan, grounded in `file:line` citations. Use before porting from an upstream or touching the engines, sessions, or the controller. -- `/upstream-audit` — compare against FlareSolverr and Byparr, classify every difference as covered, missing, or deliberate, and check `/v1` compatibility. Updates the sync ledger. -- `/port-scan` — manager for the upstream port loop. Triages new Byparr and FlareSolverr commits into labeled issues. No file-writing tools by design. `--dry-run` files nothing. -- `/audit-scan` — manager for the audit and bug-fix loop. Audits one dimension per run and files only findings that survived an attempt to refute them. Same containment. `--dry-run` files nothing. -- `/loop-work` — the worker both managers feed. Takes one `loop:ready` issue, works it in its own worktree and branch, fixes every site the issue lists, proves it with three gates, opens a draft PR. Never merges. `--dry-run` mutates nothing. -- `/live-check` — verify a change against live challenges through an isolated container. The unit tests cannot tell you whether a page still clears; this can. -- `/release` — cut a version end to end: decide the bump, preflight, tag, then verify the workflows and the published image digests. -- `/session-handoff` — rewrite `Handoff.md` from verified state, then bring the CHANGELOG, dependent docs, and memory store in line with it. -- `/pr-review` — review changes via the four specialist agents in parallel. -- `/tighten` — trim verbose docs and WHAT comments without losing vital info. Always plans first. -- `/context-budget` — what this `.claude/` config costs per turn. +- `/scout`: investigate one non-trivial task, then produce its plan, grounded in `file:line` citations. Use before porting from an upstream or touching the engines, sessions, or the controller. +- `/code-research`: fan-out research for a broad question spanning many files. `/scout` is for one concrete task. +- `/deep-audit`: read-only, many-agent audit of a whole range (default the branch against `main`) before a PR or a release. Maps the range and stops for approval, runs fourteen lenses including engine parity and two-ends tracing, refutes every finding before it counts, mutation-checks the range's new tests, and reports. Refuted findings go in its ledger. It never fixes anything. +- `/upstream-audit`: compare against FlareSolverr and Byparr, classify every difference as covered, missing, or deliberate, and check `/v1` compatibility and the dependency pins. Proposes the ledger update. +- `/live-check`: verify a change against live challenges through an isolated container. The unit tests cannot tell you whether a page still clears; this can. +- `/release`: cut a version end to end: decide the bump, preflight, tag, then verify the workflows and the published image digests. +- `/session-handoff`: rewrite `Handoff.md` from verified state, then bring the CHANGELOG, dependent docs, and memory store in line with it. +- `/pr-review`: review changes via the four specialist agents in parallel. +- `/tighten`: trim verbose docs and WHAT comments without losing vital info. Always plans first. +- `/context-budget`: what this `.claude/` config costs per turn. ## Don'ts diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..41e4356 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,11 @@ +# Code of Conduct + +Solverr is a small personal project. The expectations are simple: + +- Be respectful in issues and pull requests, and assume good faith. +- No harassment, hate speech, personal attacks, or sharing anyone's private information. +- Keep it on-topic and constructive. + +The maintainer may edit, remove, or lock any comment, and may block accounts that don't follow this, at their discretion. + +To report a problem, contact [@unseensnick](https://github.com/unseensnick). If it's sensitive, reach out privately rather than opening a public issue. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..a60d365 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,85 @@ +# Contributing to Solverr + +Solverr is a personal fork, maintained in spare time, so a pull request may sit for a while or come back with changes. Bug reports are always useful. For anything bigger than a small fix, open an issue first so the approach can be agreed on before you put in the work. + +Everyone taking part is expected to follow the [Code of Conduct](CODE_OF_CONDUCT.md). + +## Reporting a bug + +Use the [bug report form](https://github.com/unseensnick/Solverr/issues/new?template=bug_report.yml). Say which image tag you run, which engine was involved, and whether you use a proxy, and attach a log taken with `LOG_LEVEL=debug`. Check the README's Troubleshooting section first: a site that blocks your IP address fails on every solver, and a residential proxy fixes that where no code change can. + +## Setting up + +You need [uv](https://docs.astral.sh/uv/) and Git. Solverr does not use a system Python; everything runs through uv. + +```bash +git clone https://github.com/unseensnick/Solverr.git +cd Solverr +git config core.hooksPath .githooks +uv venv --python 3.14 +uv pip install -r requirements.txt -r test-requirements.txt +``` + +The `core.hooksPath` line turns on the same commit checks CI runs, so a problem shows up when you commit instead of after you push. To run the full service with both browsers, use Docker: `docker compose up -d --build`. + +## Running the tests + +```bash +PYTHONPATH=src uv run --no-project python -m unittest discover -s src -p 'test_*.py' -t src +``` + +This is the browser-free suite. It takes seconds, and CI runs it on every pull request. It cannot tell you whether a page still clears a real challenge, so if your change touches solving, say in the pull request what you tried against a live site. The maintainer runs a live check before anything touching solving is released. + +## The two engines + +Every request is served by one of two engines: `chrome` (Selenium with undetected-chromedriver, from FlareSolverr) or `stealth` (Camoufox, from Byparr). The rule that keeps them from drifting apart: + +- **A change a client can notice lands for both engines in the same pull request.** That covers the `/v1` response, request parameters, and what a configuration variable does. The only exception is a browser-automation capability one engine genuinely does not have; name it in the pull request. +- **Put a shared rule in the shared code, not in one engine.** `src/assembly.py`, `src/pipeline.py`, `src/budget.py` and `src/sessions.py` hold what both engines must do the same way. +- **Test it once for both.** A behaviour both engines must share gets a test in `src/test_engine_conformance.py`, which runs it against each engine. + +The full rule, with its reasoning, is [.claude/rules/engine-layer.md](.claude/rules/engine-layer.md). + +## Keeping the API compatible + +Solverr is a drop-in replacement for FlareSolverr, so clients built for FlareSolverr must keep working: + +- Add optional fields only. Never rename, retype, or remove a field in the request or the response. +- Keep the `"FlareSolverr is ready!"` banner exactly as it is. Clients detect session support by it. +- Only `http://` and `https://` URLs may reach a browser. + +## Commit messages + +CI checks every commit in a pull request with [.githooks/commit-msg](.githooks/commit-msg), so these are hard requirements, not style advice: + +- **The subject is `type(scope): summary`.** The type is one of `feat`, `fix`, `docs`, `chore`, `refactor`, `test`, `perf`, `build`, `ci`, `style`, `revert`. The scope is optional and names the area (`chrome`, `stealth`, `sessions`, `api`, `docker`). Write the summary in the imperative, in lower case, with no trailing period. +- **The subject is at most 72 characters.** +- **No em dash anywhere in the message.** Use commas, parentheses, periods, or colons. +- **No bare `#123`.** It silently links to an issue in this repository. Write `owner/repo#123` instead, for example `FlareSolverr/FlareSolverr#1626`. +- **No AI attribution.** No `Co-authored-by` trailer naming an AI tool and no "Generated with" footer. A `Co-authored-by` trailer for a person is fine. +- **No names of the sites you point Solverr at.** Write `example-site.tld` or "a Cloudflare-gated site" instead. The check also rejects a few words that would make Solverr read as a tool built for harvesting one site; `.githooks/commit-msg` lists them. +- **A change that is more than a one-liner gets a body.** Lead with one or two plain sentences on what changed and why it matters, then bullets. + +For example, `Fixed the cookie bug.` is rejected, and `fix(stealth): accept a cookie without a domain` passes. + +If a commit is rejected, reword it with `git commit --amend`, or `git rebase -i` for an older one. If you would rather not, say so in the pull request: the maintainer can reword it when merging, and your name stays on the commit. + +## The CHANGELOG + +Add a bullet to `CHANGELOG.md` under `## [Unreleased]` only when someone running Solverr, or calling its API, could notice the change. Put it under `Additions`, `Changes`, or `Fixes`, and lead with a bold headline that says what the user gets and ends in a period. The bold headline is the entire release note, so anything a deployer must act on (a new variable, a changed default) goes inside it. Tests, CI, documentation and tooling changes get no entry. If you are unsure, leave it out and the maintainer will add it. + +## Upstream code + +Solverr still takes changes from both of its upstreams, [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) and [Byparr](https://github.com/ThePhaseless/Byparr). [docs/dev/upstream-sync.md](docs/dev/upstream-sync.md) records what has been taken and what is deliberately different; read it before porting something or calling a difference a bug. Some files are kept byte-identical to FlareSolverr so they stay mergeable, and should not be edited: `src/undetected_chromedriver/`, `src/tests.py`, `src/tests_sites.py`, `src/bottle_plugins/`, and `html_samples/`. + +## How pull requests are merged + +The maintainer merges with a merge commit, so your commits land on `main` as you wrote them, under your name. + +## Claude Code configuration + +`CLAUDE.md` and `.claude/` configure [Claude Code](https://claude.com/claude-code) for this repository. You do not need them to contribute; they hold the same rules as this file, in more detail. + +## License + +Solverr is licensed under the [GNU General Public License v3.0](LICENSE), and contributions are accepted under the same license. diff --git a/README.md b/README.md index 06e6b74..2ada38e 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,8 @@ Solverr is a proxy server to bypass Cloudflare and DDoS-GUARD protection. It fuses the two best open-source solvers into one service and switches between them automatically, so you get reliable solving **and** coverage of the newer challenge tiers. -- **Chrome engine** (default) — the original [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) approach: [Selenium](https://www.selenium.dev) + [undetected-chromedriver](https://github.com/ultrafunkamsterdam/undetected-chromedriver) driving a real Chromium. Fast, session-capable, and clears most sites. -- **Stealth engine** — [Byparr](https://github.com/ThePhaseless/Byparr)'s stack: [Camoufox](https://github.com/daijro/camoufox) (an anti-detect Firefox that patches its fingerprint in compiled code) + [playwright-captcha](https://github.com/techinz/playwright-captcha). Clears the newer Cloudflare **Turnstile / Managed Challenges** that headless Chromium gives up on. +- **Chrome engine** (default): the original [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) approach: [Selenium](https://www.selenium.dev) + [undetected-chromedriver](https://github.com/ultrafunkamsterdam/undetected-chromedriver) driving a real Chromium. Fast, session-capable, and clears most sites. +- **Stealth engine**: [Byparr](https://github.com/ThePhaseless/Byparr)'s stack: [Camoufox](https://github.com/daijro/camoufox) (an anti-detect Firefox that patches its fingerprint in compiled code) + [playwright-captcha](https://github.com/techinz/playwright-captcha). Clears the newer Cloudflare **Turnstile / Managed Challenges** that headless Chromium gives up on. It speaks the exact FlareSolverr `/v1` API on port `8191`, so it is a drop-in replacement: existing clients (the *arr stack, manga/novel readers, etc.) work unchanged. @@ -13,9 +13,9 @@ Beyond the two engines, it keeps **[sessions](#sessions--automatic-cleanup)** wa ## Contents -- **Getting started** — [How it works](#how-it-works) · [Quick start](#quick-start) · [Installation](#installation) -- **Using it** — [Engines & fallback](#engines--fallback) · [Sessions & cleanup](#sessions--automatic-cleanup) · [API usage](#api-usage) · [Passthrough proxy](#passthrough-proxy) -- **Reference** — [Configuration](#configuration) · [Proxy & reliability](#proxy--reliability) · [Prometheus exporter](#prometheus-exporter) · [Troubleshooting](#troubleshooting) +- **Getting started**: [How it works](#how-it-works) · [Quick start](#quick-start) · [Installation](#installation) +- **Using it**: [Engines & fallback](#engines--fallback) · [Sessions & cleanup](#sessions--automatic-cleanup) · [API usage](#api-usage) · [Passthrough proxy](#passthrough-proxy) +- **Reference**: [Configuration](#configuration) · [Proxy & reliability](#proxy--reliability) · [Prometheus exporter](#prometheus-exporter) · [Troubleshooting](#troubleshooting) ## How it works @@ -85,18 +85,19 @@ On a Debian **host**, make sure `libseccomp2` is 2.5.x (`sudo apt-cache policy l ### From source -For development or unsupported architectures. Requires Python 3.9+ (3.11+ recommended for the vendored undetected-chromedriver; the Docker image uses 3.14), and both browsers if you want both engines: +For development or unsupported architectures. Needs [uv](https://docs.astral.sh/uv/) and Python 3.14 (the version the image runs), plus both browsers if you want both engines: ```bash -# install Python deps (pip, or `uv pip`) -pip install -r requirements.txt +# create the environment and install Python deps +uv venv --python 3.14 +uv pip install -r requirements.txt # Chrome engine: install Chrome or Chromium (+ Xvfb on Linux) # Stealth engine: install Firefox libraries and fetch Camoufox -playwright install-deps firefox -python -m invisible_playwright fetch +uv run --no-project playwright install-deps firefox +uv run --no-project python -m invisible_playwright fetch -python src/flaresolverr.py +uv run --no-project python src/flaresolverr.py ``` Set `STEALTH_ENGINE=false` to run Chrome-only and skip the Camoufox/Firefox setup entirely. @@ -126,7 +127,7 @@ Clients often create a session and never destroy it (a mobile app can be killed - closes any session idle longer than `SESSION_TTL_MINUTES` (default 30). Every request bumps the session's last-used time, so an in-use session is never reaped. - evicts the oldest-idle session once an engine exceeds `SESSION_MAX` (default 20). -So `sessions.destroy` is good practice but optional — cleanup happens automatically. +So `sessions.destroy` is good practice but optional: cleanup happens automatically. ## API usage @@ -232,7 +233,7 @@ Like `request.get`, plus `postData`. ## Passthrough proxy -Some clients don't consume the solved HTML that `/v1` returns. Instead they take the `cf_clearance` cookie and **re-fetch the URL themselves** with their own HTTP client. Cloudflare fingerprints that second request (different TLS/JA4, HTTP/2 settings, headers) than the browser that solved the challenge, decides it doesn't match, and re-challenges — so the client fails even though the solve worked. Indexer managers that drive Cloudflare-protected sites are the common case. +Some clients don't consume the solved HTML that `/v1` returns. Instead they take the `cf_clearance` cookie and **re-fetch the URL themselves** with their own HTTP client. Cloudflare fingerprints that second request (different TLS/JA4, HTTP/2 settings, headers) than the browser that solved the challenge, decides it doesn't match, and re-challenges, so the client fails even though the solve worked. Indexer managers that drive Cloudflare-protected sites are the common case. The passthrough removes the replay step. Point the client at Solverr's passthrough port instead of the site; Solverr solves in-process (reusing engine fallback, sessions, and per-host memory) and returns the solved body as a clean `200`. The client never sees a challenge, so it never re-fetches. @@ -276,14 +277,14 @@ You don't need a bundled indexer file. Take the site's existing definition from - **Prowlarr**: `/config/Definitions/Custom/`. The `Custom` subfolder often doesn't exist yet, and Prowlarr **ignores** YAMLs placed directly in `Definitions/`, so create `Custom/` and put the file there. - **Jackett**: its custom-definitions folder, which Jackett prints in its startup log (commonly `/config/Jackett/Indexers/custom/` on the linuxserver image); create it if missing. -Then add the indexer in the manager, pick a mirror as the **Base URL**, and **do not attach a FlareSolverr/proxy tag** — the passthrough already does the solving, and a proxy tag would route around it. Everything else in the definition (search paths, selectors, categories) stays untouched. +Then add the indexer in the manager, pick a mirror as the **Base URL**, and **do not attach a FlareSolverr/proxy tag**: the passthrough already does the solving, and a proxy tag would route around it. Everything else in the definition (search paths, selectors, categories) stays untouched. > **Grab the definition as a file, not via copy-paste.** A few definitions contain non-printable characters in their filters (a rare title-cleanup step); pasting through a chat or some editors silently strips them and breaks parsing ("No title provided" on every result). Download the raw file so the bytes stay intact. Notes and limits: - **`GET`/`HEAD` only**; request bodies aren't forwarded. Most indexer definitions are `GET`. -- Encode the mirror as a **bare host** (`example-site.tld`), not `https://…` — clients that normalise `//` in a path would otherwise corrupt an embedded scheme. +- Encode the mirror as a **bare host** (`example-site.tld`), not `https://…`, because clients that normalise `//` in a path would otherwise corrupt an embedded scheme. - Successful bodies are cached for `PASSTHROUGH_CACHE_TTL`; challenge pages and non-2xx responses are not, so a transient block retries rather than sticking. - The cache holds at most `PASSTHROUGH_CACHE_MAX_BYTES` in total. The TTL alone bounded how long a body was kept but not how much was kept, so a client walking many pages inside one TTL window could hold all of them at once. - It's still bound by IP reputation like any solve (see [Proxy & reliability](#proxy--reliability)). If a site blocks your IP, a residential `PROXY_URL` applies to passthrough solves too. @@ -404,7 +405,7 @@ If the exit IP can't be reached, Solverr falls back to the container's `TZ` for ## Proxy & reliability -No solver beats Cloudflare by fingerprint alone — **IP reputation dominates**. A datacenter/VPS IP fails far more challenges than a residential one. If a site keeps failing on **both** engines, the single most effective fix is a residential proxy: set `PROXY_URL` (and credentials), or pass `proxy` per request/session. +No solver beats Cloudflare by fingerprint alone: **IP reputation dominates**. A datacenter/VPS IP fails far more challenges than a residential one. If a site keeps failing on **both** engines, the single most effective fix is a residential proxy: set `PROXY_URL` (and credentials), or pass `proxy` per request/session. Rough guide to expected latency: Chrome solves take a few seconds; Camoufox solves take ~10–20 s (the price of clearing challenges Chromium can't). Session reuse brings follow-ups on the same host down to ~1–3 s. @@ -416,7 +417,7 @@ The domain label is capped at 100 distinct hosts; every host after that is repor ## Troubleshooting -**A source shows no results but the log says `Challenge not detected!` with a 200.** An engine loaded the page but couldn't recognise a newer managed/Turnstile challenge and returned it as if solved. Solverr's auto-fallback is designed to catch this and retry on the other engine; make sure `ENGINE_FALLBACK` is on and the stealth engine is enabled. If it still fails, the site is likely gating on your IP — add a residential proxy. +**A source shows no results but the log says `Challenge not detected!` with a 200.** An engine loaded the page but couldn't recognise a newer managed/Turnstile challenge and returned it as if solved. Solverr's auto-fallback is designed to catch this and retry on the other engine; make sure `ENGINE_FALLBACK` is on and the stealth engine is enabled. If it still fails, the site is likely gating on your IP, so add a residential proxy. **Out-of-memory / browser launch errors (Proxmox LXC, low-RAM hosts).** Give the container more shared memory: `shm_size: 512mb` in `docker-compose.yml` (or `--shm-size=512m`). Reduce `SESSION_MAX` and keep `SESSION_TTL_MINUTES` modest so idle browsers are freed sooner. @@ -424,6 +425,10 @@ The domain label is capped at 100 distinct hosts; every host after that is repor **Cloudflare has blocked this request / IP banned.** Your IP is flagged for that site. Try a (residential) proxy, or open the site in a normal browser from the same network to confirm. +## Contributing + +Bug reports and pull requests are welcome. Read [CONTRIBUTING.md](CONTRIBUTING.md) before opening a pull request: it covers the setup, the tests, and the commit message standard that CI checks every commit against. Everyone taking part is expected to follow the [Code of Conduct](CODE_OF_CONDUCT.md). + ## License Solverr is licensed under the **GNU General Public License v3.0** (see [LICENSE](LICENSE)). It began as a fork of [FlareSolverr](https://github.com/FlareSolverr/FlareSolverr) (MIT) and its stealth engine derives from [Byparr](https://github.com/ThePhaseless/Byparr) (GPL-3.0); because Byparr is copyleft, the combined work is GPL-3.0. Upstream copyright notices are preserved in [NOTICE](NOTICE). diff --git a/docs/dev/engine-layer-architecture.md b/docs/dev/engine-layer-architecture.md index 185020d..e908db2 100644 --- a/docs/dev/engine-layer-architecture.md +++ b/docs/dev/engine-layer-architecture.md @@ -246,11 +246,11 @@ copy cannot drift from the law. `Reactor` that ships alongside it is deliberately not used, since it starts a polling thread per driver. Three caveats bound the work: the capability is set at driver creation so it is a config decision rather than a per-request one, the log must be drained per request or it grows on a - long-lived session, and it needs a fingerprint A/B before shipping because it is on by default. -- **Identity is a sealed `SessionRef`, not a bare id string.** The two pools can hold the same id and - `_cmd_sessions_list` already deduplicates them at runtime, which is the symptom. A bare string - cannot express which engine owns a session, so the wrong-pool lookup stays constructible until the - type says otherwise. + long-lived session, and it needs a fingerprint A/B before it could ever be on by default. It + shipped off by default for that reason. +- **Identity stays a bare id string.** A sealed `SessionRef` was proposed because the two pools can + hold the same id, and dropped once measured: the `/v1` contract has clients send a bare id, and + resolving which engine holds one is the controller's job. Step 5 above records the full reason. - **The conformance suite is the pin, and the spine is the kernel.** Both rungs of the ladder are available here because the codebase is small enough, so an unpinned twin should not exist at all. - **Sequencing puts characterisation second, not last.** `/live-check` is user-invoked and takes tens diff --git a/docs/dev/loops.md b/docs/dev/loops.md deleted file mode 100644 index b69ec18..0000000 --- a/docs/dev/loops.md +++ /dev/null @@ -1,138 +0,0 @@ -# The loops - -Work on Solverr has two recurring shapes. An upstream moves and someone decides what applies here. Or something in this repo is wrong and someone finds it, proves it, and fixes it. Both end the same way: a branch, a verification pass, a PR a person reviews. - -This file defines both as loops with an explicit contract, so they run on a schedule instead of only when someone remembers. - -A loop is recurring work with five parts: a **job**, **permissions**, a **schedule**, **state that outlives the conversation**, and an **evaluation**. Three of those already existed here. `docs/dev/upstream-sync.md` is the state for the port loop, GitHub issues are the state for both. `/live-check` is the evaluation. The commit hooks, the pre-commit lint, and the Standards CI job are the permissions boundary. The loops add the job definitions and the schedule. - -## Shape: two managers, one worker - -``` -/port-scan (upstream moved) ─┐ - ├─> labeled issues ─> /loop-work ─> draft PR ─> a person merges -/audit-scan (defect here) ─┘ -``` - -The managers decide what should be done and have no file-writing tools. The worker does one thing at a time and cannot merge it. They never talk to each other directly; they talk through issues and labels, which is also where a person can see and change the queue. - -A single skill that both triaged and implemented would be simpler and would lose the only structural check in the design: bad triage would go straight to a branch. Splitting it means a wrong call costs a wrongly labeled issue, which is one glance to fix. - -One worker rather than two, because the mechanics after triage are identical: worktree, branch, gates, draft PR. Only the provenance differs, and a `source:` label carries that. - -Accountability sits with the person operating this. Every merge is a human decision. - -## Manager: `/port-scan` - -- **Job**: notice that an upstream moved, and itemise which of it applies here. -- **Inputs**: `../Byparr` and `../FlareSolverr` (fetched, never checked out), `docs/dev/upstream-sync.md`, existing issues, `Handoff.md`. -- **Allowed**: create and label issues, create labels. -- **Forbidden**: any file write, any branch, any PR, any change to the ledger. -- **Output**: one issue per portable upstream commit, labeled `source:upstream` plus `loop:ready` or `loop:needs-human`, with the upstream SHA in the title and every affected site enumerated. -- **Evaluation**: a repeat run with nothing new upstream files nothing. That is the loop's own regression test. -- **Escalation**: anything it cannot classify confidently becomes `loop:needs-human`. - -## Manager: `/audit-scan` - -- **Job**: audit one dimension of this repo per run and itemise the defects that survive an attempt to refute them. -- **Inputs**: one dimension of `src/`, `CLAUDE.md`'s recorded decisions, the ledger's divergences, prior `source:audit` issues including closed ones, `Handoff.md`, recent commits. -- **Allowed**: create and label issues, run the browser-free suite to prove a finding. -- **Forbidden**: any file write, any branch, any PR. No Docker and no browser, which is why anything needing a live solve to prove goes to a human. -- **Output**: one issue per defect (never one per site), labeled `source:audit` plus `loop:ready` or `loop:needs-human`, carrying a failure scenario, every affected site, and the refutation attempt that failed to kill it. -- **Evaluation**: a run that refutes everything it found is a successful run. A loop that manufactures findings to look productive is the failure mode. -- **Escalation**: unproven without a browser, or unsure, both become `loop:needs-human`. - -**Why the refutation gate is the centre of this loop.** An earlier audit reported `postform.py`'s percent-encoding as a double-encoding bug. The reasoning was clean and it was wrong: the form travels as a `data:text/html,` URL, so the browser URL-decodes before the HTML parser runs, and removing `quote()` broke POST for values holding `%` or `#`. A live A/B caught it, after the finding was already written up. Code that looks wrong in this repo usually encodes something that was measured, so a finding must answer the recorded reason rather than not notice it. - -## Worker: `/loop-work` - -- **Job**: land exactly one `loop:ready` issue as a draft PR carrying its own evidence. -- **Inputs**: one issue, the repo at `origin/main`, and whatever the issue names. -- **Allowed**: a worktree under `.worktrees/`, a `loop/*` branch, edits under `src/`, tests, `CHANGELOG.md`, the ledger (for `source:upstream` only), throwaway containers suffixed `-loop`, a push of its own branch, a draft PR. -- **Forbidden**: merging, marking a PR ready, pushing to `main`, force-pushing, tagging, releasing, taking a second issue, taking a `loop:needs-human` issue, bypassing a hook, touching the sibling clones. -- **Output**: a draft PR with three gates reported as numbers and the site checklist marked off, and the issue relabeled `loop:in-review`. -- **Evaluation**: the three gates below. -- **Escalation**: `/scout` finding the change bigger than the issue said, a live tally clearly worse than baseline, or a High review finding whose fix would reach a tripwire zone. - -## Scope, and why eligibility does not count files - -The first draft of these rules made `loop:ready` mean "at most one file under `src/`". That is exactly the rule that produces patchwork: it rewards fixing the one call site that made the bug visible and leaving its siblings alone, and it makes a well-understood four-file change ineligible while an unexamined one-file change sails through. - -Eligibility is about **how well the scope is known**, never how small it is. Every issue carries a checklist of every affected site with `file:line`, plus the search that produced it, and the worker re-runs that search before calling the code done. Three outcomes are acceptable: fix every site, fix some and list the rest in the PR body with the reason, or escalate. Silence about a site is not one of them. - -Solverr has two engines written against each other, so a defect in one usually has a twin in the other. That is the single most common way a fix here ends up half done. - -Refactoring is in scope when the correct fix needs it, in the same change, with the reason in the commit body. What stays out is adjacent cleanup nothing in the issue motivates. See `.claude/rules/code-quality.md`. - -## The three gates - -Cheapest first, so a run fails fast. - -**Gate A, the browser-free suite.** 138 tests, no browser, seconds. Pass or fail, no interpretation. - -**Gate B, the live solve tally.** The one gate that cannot be boolean. Cloudflare's behavior varies with IP reputation, time of day, and how hard a host was hit five minutes ago, so a single result carries no information either way. The worker builds a baseline container off `origin/main` and runs it interleaved with the change, trial for trial, in the same window. Within noise is a pass, clearly worse is a fail, and an ambiguous window opens the PR labeled `needs-live-recheck` with the raw numbers. Asking a person is a valid outcome; a confident verdict off one sample is not. - -**Gate C, the consuming chain.** Throwaway containers on a private network, on non-default ports, running one search. Prowlarr disables an indexer after about 100 seconds, so a slow solve reads as a broken indexer; on a throwaway instance that costs nothing. - -There are two chains and picking the wrong one makes the gate meaningless. Solverr's own passthrough (`src/passthrough.py`) and `byparr-proxy` are separate implementations of the same idea, and `byparr-proxy` fronts `/v1`, so it never runs a line of the passthrough. A change to one is proved by driving that one. This bit on the first real run: the gate as originally written would have passed a passthrough change without executing any of it. - -A gate on a bound also needs the bound set small enough to reach in a short run, sized against a measured response rather than a guessed one. A production-sized default is never hit in five requests, and the pass looks identical either way. - -## After several land: tidy the changelog - -Each worker branches from `main` and adds its CHANGELOG bullet under a category heading. Branches cut before their siblings merged each create their own heading, and git merges them cleanly as separate blocks, so three merged PRs leave three `### Changes` headings stacked under `## [Unreleased]`. Nothing warns about it: it is not a conflict, and the pre-commit lint checks entry format rather than section structure. - -The worker cannot prevent this, since it cannot see branches that have not merged yet. Fold the duplicate headings into one on `main` after a batch lands, before the next release cut. Ordering the bullets by audience size (everyone, then one subsystem's users, then an opt-in feature's) reads better than merge order. - -## Labels - -| Label | Set by | Meaning | -|---|---|---| -| `loop:ready` | either manager | The worker may take this unattended. | -| `loop:needs-human` | either manager, or the worker on escalation | Real work, but not alone. | -| `loop:in-review` | worker | A draft PR exists. | -| `source:upstream` | `/port-scan` | Came from an upstream commit. | -| `source:audit` | `/audit-scan` | Came from a verified defect here. | -| `needs-live-recheck` | worker | Gate B was ambiguous. Re-run the tally before merging. | - -Dedupe on the `source:` labels, never on the `loop:` ones. Repeated `--label` flags in `gh` are ANDed, so listing several `loop:` states at once matches nothing, and a manager reading that as "nothing filed yet" re-files everything on every run. `loop:in-review` also stays on an issue after its PR merges, as a record of how it was handled; nothing reads it once the issue is closed. - -## Tripwire zones - -`loop:needs-human`, always, whichever manager finds it: the widget measuring and click path in `stealth_engine.py`, the shared `maxTimeout` budget split, the `quote()` calls in `postform.py`, session and reaper lifecycle, `geo.py`, and any dependency pin for the browser stack. `Handoff.md`'s "What failed" section is the list of conclusions a confident agent reaches and gets wrong, so it is also the list of things the worker may not reason about alone. - -## Guards - -The worker can write code and push a branch, so the guards around it are worth stating. - -`.claude/hooks/block-dangerous-commands.sh` blocks pushes to protected branches, force pushes, and destructive operations. It matches **both** the Bash and the PowerShell tool: matching only Bash left every guard bypassable by rewriting the same command in PowerShell, which is a different tool with a different name and its own spelling for every destructive operation. Fixtures under `.claude/hooks/tests/fixtures/` cover both syntaxes; run them with `bash .claude/hooks/tests/run-all.sh`. - -The commit-msg and pre-commit hooks are never bypassed. `--no-verify` is not an option the worker has. - -## Running them - -Dry-run first, always, on any shape the loop has not seen: - -```bash -/audit-scan --dry-run sessions -``` - -Then the real scan, and the worker on whatever it queued: - -```bash -/loop-work --dry-run 12 -``` - -On a schedule, one at a time, with `/loop`: - -```bash -/loop 1d /port-scan -``` - -Cadence follows the input, not the calendar. Byparr moves in bursts (30 commits across two days in August 2026, then nothing) and FlareSolverr is close to dormant (one commit in July 2026, months of gaps before it), so daily is right for `/port-scan` and cheap when the range is empty. `/audit-scan` is bounded by its dimension list rather than by upstream activity, so weekly gets through the rotation without re-treading ground. - -Both run locally and only while the machine is on. Gates B and C need Docker and a real network path to live challenges, so a cloud scheduled agent cannot serve as the worker. - -## What these loops do not do - -They do not merge, release, or decide that a divergence should end. They do not touch the engines without a person in the path. They do not run the paid CAPTCHA escalation, which needs an API key, and they do not cover a Cloudflare-gated PDF, because no such URL has been found. Those stay in the "not covered" section of every PR body the worker writes. diff --git a/docs/dev/upstream-sync.md b/docs/dev/upstream-sync.md index 0419c57..330a72f 100644 --- a/docs/dev/upstream-sync.md +++ b/docs/dev/upstream-sync.md @@ -37,7 +37,7 @@ Cite one of these instead of re-arguing it. Change one only when the owner asks. - **`LANG` rather than a new `BROWSER_LOCALE`.** Byparr added `BROWSER_LOCALE` (`8cb5770`) because it had no language variable. Solverr inherited `LANG` from FlareSolverr, wired to Chrome's `--accept-lang` (`src/utils.py`), so a second variable would have meant one knob per engine. `LANG` now feeds both through `config.browser_locale()`. It is also normalized rather than forwarded: `invisible_core/prefs.py` only maps `_` to `-` and appends the base subtag, so a raw `LANG=en_US.UTF-8` would set `navigator.languages = ["en-US.UTF-8", "en"]` and a matching `Accept-Language`, which is a more distinctive fingerprint than leaving it unset. Values that are not language tags are dropped with a warning. - **The browser language comes from the same lookup as the timezone.** Byparr resolves a locale from the exit country and Chrome has never resolved one at all, so with nothing configured the two engines answered in different languages (measured 2026-08-13: Chrome `en-US,en`, Camoufox `nb-NO,nb` from the same Norwegian exit), and `ENGINE_FALLBACK` made it change mid-session. Neither value was wrong; disagreeing was. `src/geo.py` now takes the exit IP out of `prepare_session_geo` and feeds it to `resolve_session_locale`, so both are derived from one address and cannot name different countries. That is one round trip behind a proxy and two on a direct connection, where the library reports no `egress_ip` (the field exists for the WebRTC override) and the locale resolver looks the address up again; the result is cached per proxy either way, so it is per process rather than per launch. That pairing is the point: the library's own `_warn_locale_fallback` comment says a locale falling back to `en-US` while the timezone resolves is "a cross-field inconsistency of exactly the kind the timezone trap exists to prevent". Chrome also gets the `tag, base` pair rather than a bare tag, since `--accept-lang` is passed through verbatim and produced a one-entry `navigator.languages` no desktop browser sends. - **Solverr resolves the browser timezone itself, for both engines.** Byparr leaves it to `invisible_playwright`, which resolves from the exit IP on every launch inside the library. Measured on 2026-08-13 against the pinned version: an address lookup under a 15 second budget plus a 53 MB geoip download, 8 seconds cold, no caching anywhere in `invisible_core/_geo.py`, and behind a proxy `prepare_session_geo` raises on a failed lookup and takes the launch with it. Solverr resolves in `src/geo.py`, once per proxy and cached, and passes a concrete `timezone=`, which returns from `prepare_session_geo` before that fatal branch. Behind a proxy the library still makes one lookup of its own for the WebRTC override, but non-fatally. Chrome gets the same zone through `Emulation.setTimezoneOverride`, chosen over a JS patch because it moves the browser's own ICU clock and leaves nothing in the document looking rewritten, and over the `TZ` environment variable because `uc.Chrome()` exposes no per-process environment and a process-global would race concurrent launches on different proxies. FlareSolverr has no timezone handling at all, so nothing to reconcile there. -- **`STEALTHFOX_GEOIP_MMDB` is set in the image, and it is not a public API.** The geoip database is baked at build time and pinned with this variable, because `ensure_geoip_mmdb` re-checks for a newer build on every call and would download over the baked copy. The variable belongs to `invisible_core` 18.13.0, held by the exact `invisible-playwright==0.6.1` pin, so re-check the name whenever that pin moves. It is the only thing here depending on a dependency's internals. +- **`STEALTHFOX_GEOIP_MMDB` is set in the image, and it is not a public API.** The geoip database is baked at build time and pinned with this variable, because `ensure_geoip_mmdb` re-checks for a newer build on every call and would download over the baked copy. The variable belongs to `invisible_core` (20.15.0, held by the exact `invisible-playwright==0.7.2` pin), so re-check the name whenever that pin moves. It is the only thing here depending on a dependency's internals. - **`BROWSER_GEO` reads the system tzdata table, with four overrides.** Country to timezone comes from `/usr/share/zoneinfo/zone1970.tab` rather than a list kept in the repo, since a hand-kept list is how Camoufox ended up returning wrong zones (`daijro/camoufox#589`). Two things measured against the real file on 2026-08-13 that its header does not prepare you for: a row lists every country sharing a zone and only the first is the one it speaks for (crediting them all put Germany in `Europe/Zurich`, because the Swiss row sorts first), and the promise to put "the most populous timezones first" is overridden by geography for 4 of the 24 multi-zone countries, which tzdata orders east to west. `_POPULATION_ZONES` in `src/geo.py` corrects Brazil, Russia, Australia and Canada, and an override applies only when the system confirms the zone exists. - **`BROWSER_WAIT_TIMEOUT` lives in `config.py` and falls back instead of raising.** Upstream put `get_config_browser_wait_timeout` in `utils.py` and parses with a bare `int()`, which raises on a value that is not a number. Two reasons not to copy that. It is read inside `_evil_logic`, so upstream's raise happens mid-solve, where the engine wraps it as "Error solving the challenge", the controller falls back to the other engine, and nobody learns the variable is malformed; `config._int_env` defaults to 1 instead, matching every other integer knob here. Upstream also reads it a second time at startup purely to make a bad value fatal there, and that line is unused, which this fork does not carry. The cost is that `utils.py` now diverges from upstream by one absent function on top of the `LANG` normalization, so expect that hunk when diffing. - **The Chrome Turnstile path waits for the widget and bounds its retry loop.** Upstream's `_resolve_turnstile_captcha` (`src/flaresolverr_service.py`, unchanged through `3ae649a`) reads `TURNSTILE_SELECTORS` the instant `driver.get()` returns and then retries in a `while True` with no deadline. Both halves are wrong together, and each one hid the other: `driver.get()` returns at `readyState complete`, so a widget injected by Cloudflare's api.js is not there yet, and a request that missed it answered "Challenge not detected!" with an empty token. Measured 2026-08-25 with a bare-browser probe in the container, over four samples across two demo pages: the token input appeared 0.01s to 0.92s after `get()` returned, and never on a page with no widget. Solverr waits `_WIDGET_RENDER_SECONDS` (5, the same grace the stealth engine gives through its networkidle settle), which is what makes the unbounded loop reachable often enough to matter, so the loop now takes a deadline of `maxTimeout` minus a margin and returns `None` rather than raising. Two smaller repairs ride along because they are the same twenty lines: the token is resolved after the cookie reload rather than before it (a reload replaces the document the token described), and the input is re-located on every read rather than held across passes (a re-render raised `StaleElementReferenceException` out of the whole request). Not offered upstream, by the owner's call. @@ -48,14 +48,14 @@ Cite one of these instead of re-arguing it. Change one only when the owner asks. ## Inherited from FlareSolverr, byte-identical -These need no review until upstream changes them. Verified identical on 2026-07-25: +These need no review until upstream changes them. Verified identical on 2026-09-11: - `src/undetected_chromedriver/` (the whole vendored package) - `src/tests.py`, `src/tests_sites.py` (they carry upstream's site list; leave as-is so the files stay mergeable, and do not add to them) - `html_samples/*.html` -- `src/bottle_plugins/` +- `src/bottle_plugins/`, except `prometheus_plugin.py`, which caps the domain label (`_MAX_DOMAIN_LABELS`) so an unbounded set of hosts cannot grow the Prometheus registry without limit -`src/utils.py` was on this list until 2026-08-13 and now carries two divergences: the `LANG` normalization below (`get_webdriver`, four comment lines and one call), and the absence of upstream's `get_config_browser_wait_timeout`, which lives in `config.py` here for the reason given below. Everything else in it is still upstream's, so diff it with `--strip-trailing-cr` before assuming otherwise: the working copy is CRLF and FlareSolverr's is LF, so a plain `diff` reports every line as changed. +`src/utils.py` was on this list until 2026-08-13 and now carries two divergences: the `LANG` normalization below (`get_webdriver`, four comment lines and one call), and the absence of upstream's `get_config_browser_wait_timeout`, which lives in `config.py` here for the reason given below. Everything else in it is still upstream's. Diff it with `--strip-trailing-cr` before assuming otherwise: `.gitattributes` now checks text out as LF, like FlareSolverr, but a checkout made before it may still be CRLF, and then a plain `diff` reports every line as changed. ## Taken @@ -67,17 +67,17 @@ These need no review until upstream changes them. Verified identical on 2026-07- - **The stealth browser from PyPI, pinned exactly.** Byparr moved `invisible-playwright` off a git dependency to a PyPI floor of `>=0.6.1` (`a0c4b1d`, `e298eb8`) and its lock now resolves 0.7.2. Solverr followed to `==0.7.2` (v1.4.0), which pins `invisible-core==20.15.0` outright; `STEALTHFOX_GEOIP_MMDB`, `prepare_session_geo`, `resolve_session_locale`, `ensure_geoip_mmdb` and the `locale`/`timezone` launch arguments were all re-checked against it and are unchanged, as is the `_` to `-` locale normalization that `config._language_tag` exists to get ahead of. Solverr pins exactly rather than to a floor, for the reason the original commit pin existed: this package carries the patched Firefox, so an unattended bump changes solving behavior while the build stays green. A floor is not a pin, which is why moving it is a release with a live check attached rather than a dependency bump. That commit pin was never as reproducible as it looked either: it fixed `invisible-playwright` at 0.3.0 but left `invisible-core` to float. - **Reading the Turnstile token off the value property** (v1.4.0). Byparr reads it with `input_value` (`challenge.py`), which is `element.value`; Solverr read it with Playwright's `get_attribute`, which is `element.getAttribute` and so returns the `value` content attribute. Taken, but for a narrower reason than it first looked: the expectation was that a token assigned to `input.value` never reaches the attribute, and measured against the live widget on 2026-08-18 both reads returned the same token, so the old read was not broken for Cloudflare's own widget. It is broken for a token the paid escalation injects, which playwright-captcha assigns to the property alone (`appliers/applyCloudflareTurnstile.js`). Parity with Selenium's `get_attribute`, which returns the property, is the rest of the reason. **No user-visible effect on the free path**, so no CHANGELOG entry. -- **Cookies read after `waitInSeconds`, on both engines** (unreleased). FlareSolverr moved its cookie read below the wait and the screenshot (`3ae649a`) so a page that sets cookies from its own JavaScript during that wait is not read too early. Solverr had the same defect twice, once per engine (`chrome_engine.py`, `stealth_engine.py`), because the two were written against each other. Fixed in both. `returnOnlyCookies` never waits, so it is unaffected either way, and the read stays outside that branch so it still returns cookies. `test_response_shape.py` covers the Chrome side with a fake driver whose jar changes while the wait runs; the stealth side is the same edit against an async context and rests on the live check. -- **A single reusable focus helper in the Chrome turnstile loop** (unreleased). `_get_turnstile_token` prepended a fresh `