RELEASE-FIX-C: never enable Fetch with nothing to intercept (does NOT fix macOS — see correction) - #46
Open
AminDhouib wants to merge 5 commits into
Open
RELEASE-FIX-C: never enable Fetch with nothing to intercept (does NOT fix macOS — see correction)#46AminDhouib wants to merge 5 commits into
AminDhouib wants to merge 5 commits into
Conversation
setup_interception ran on every spawn and, when the instance had zero hooks, enabled a catch-all Fetch pattern pair anyway. Fetch.enable pauses EVERY request before it leaves the browser; only the Fetch.RequestPaused handler resumes it. On macOS under the detached backend that handler never fires, so every navigation hung -- proven in W2 CI by a probe where about:blank succeeded in 1.1s while http://127.0.0.1:1/ (a refused connection, which cannot hang) hung 35.3s and the fixture server logged 0 hits. Two halves, neither shippable alone: 1. setup_interception: when the computed pattern list is empty, log and return without calling fetch.enable. A default spawn now arms nothing. 2. create_hook: interception was established ONLY at spawn -- the catch-all was accidentally covering hooks created later, since _on_request_paused resolves hooks per event. A bare early return would silently break spawn -> create_dynamic_hook -> navigate, so create_hook now re-arms by calling the SAME setup_interception (one home, no second enabling path). setup_interception retains the tab it is handed at spawn (_instance_tabs) so the re-arm has one to use; browser_manager already passes it, so nothing new crosses a module boundary and no embedded module imports server. remove_instance drops the reference with the rest of the instance's hook state. Re-arming can now call setup_interception repeatedly, so RequestPaused is registered at most once per instance (_paused_handlers) -- a second handler would double-continue every paused request. Scope: this restores the DEFAULT (zero-hook) path. Why RequestPaused never dispatches on macOS in the detached backend is still open as F-773; a macOS user who creates a hook re-enables interception and, on current evidence, hangs again. That is not fixed here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tract
Three RED-first pins in the existing hook module, no real Chrome:
- test_no_hooks_means_no_fetch_enable: a zero-hook spawn sends nothing and
registers no handler. RED before C1 with `assert ['enable'] == []`.
- test_hooks_still_intercept: with one hook, Fetch.enable carries THAT hook's
urlPattern (not the catch-all "*"). GREEN before and after -- it is the proof
C1 disabled nothing, and the proof the harness itself works, so the other two
REDs cannot be harness errors.
- test_hook_created_after_spawn_arms_interception: the regression guard. RED
before C1 on the spawn assertion, and RED again against a bare early-return
(verified by staging half 1 alone: `assert [] == ['enable']` after
create_hook) -- exactly the silent break C1's second half exists to prevent.
Also holds that a second hook re-sends the widened pattern set without
stacking a second RequestPaused handler.
FakeTab grows two additive recorders to make this assertable: cdp_frames (the
{"method","params"} frame, by advancing the nodriver command generator once
before closing it) so a test can assert a CDP command's ARGUMENTS, and
add_handler/handlers so it can count event subscriptions. send_calls is
unchanged, so no existing test's expectations move.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ingle-variable test) FIX-C did NOT fix macOS, and the backend log proves the fix itself ran: "No active hooks ...; leaving Fetch interception disabled". With interception demonstrably OFF, a navigation to a CLOSED port still hangs 35s while about:blank -- the same Page.navigate command -- returns in 1s. So the paused-Fetch hypothesis is disproved. FIX-C remains worth keeping on its own merits (it removes a real per-request tax and a real fragility, RED-first pinned), but it is not the macOS fix. What the disproof leaves: CDP is healthy and only network-requiring navigations stall, in the detached backend, on macOS only. This tests the one concrete difference between the macOS cases that pass and the one that does not. In-process integration runs its Chrome profile under RUNNER_TEMP (/Users/runner/work/_temp) and navigates fine; this journey ran under pytest tmp, which on macOS is /private/var/folders/... -- the sandboxed per-user temp. Same OS, same Chrome, same code, different profile location. So: workspace goes under RUNNER_TEMP when set, pytest tmp_path otherwise, and the test cleans up what it created. One variable changed. If macOS goes green, that was it; if not, the location is excluded and the remaining difference is in-process vs detached, which I will route rather than keep guessing at. Local (Windows): transport passes both with and without RUNNER_TEMP set, so both branches of the helper are exercised; ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cess tree) RUNNER_TEMP made no difference -- the log confirms the workspace moved to /Users/runner/work/_temp/gate-*/ and the hang is byte-identical. Profile location is excluded, alongside cold start, dead tab, broken CDP session, reachability, and (via FIX-C) Fetch interception. On the same runner, in the same job, 53 in-process tests navigate fine. What is left is the detached backend itself. Two instruments, chosen because they answer that directly: 1. Chrome's OWN log (--enable-logging --log-file=... into log_dir, so the existing failure dump collects it). Checked against the product's stealth-arg blocklist first -- neither flag is on it, so unlike --use-mock-keychain these actually reach Chrome. Warmup instance only; the canonical journey stays a clean default spawn. 2. A snapshot of the live Chrome process tree by --type=, taken WHILE the navigation is stalled (after teardown the evidence is gone). Chrome runs its network stack in a separate utility process; whether that process EXISTS separates "cannot reach the network" from "never asked it to", and nothing at the CDP boundary can tell those apart. Human chose this instrumented round over routing it now. It is diagnosis, not another guess: both instruments report facts regardless of which way they fall, and neither can turn the gate green on its own. Local (Windows): transport passes; snapshot helper verified live (it resolves --type=utility --utility-sub-type=... entries correctly); ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AminDhouib
changed the title
Member
Author
Correction: this does NOT fix F-772/macOSThe PR description above was written before CI tested it. It is wrong on the headline claim and I am leaving it visible rather than quietly rewriting history. The fix ran exactly as designed on the macOS runner — the backend log shows What this PR is actually worth, and it is not nothing:
Judge it on those merits alone. The macOS defect continues as F-773 with a fuller elimination table; nothing in this PR should be read as evidence about macOS. |
11 CI rounds narrowed the macOS failure to a wall. finding_F773 records it properly: the symptom, the probe that bounds it, and an elimination table where every row was closed by an experiment rather than an argument -- cold start, tab/CDP health, fixture reachability, Fetch interception (disproved by shipping FIX-C), profile location (disproved via RUNNER_TEMP), the network-service process, Chrome errors, and the Mach rendezvous lines that turned out to be teardown noise 2 minutes after the fact. The decisive measurement, kept front and center: a connection to a CLOSED port hangs 35s while about:blank returns in 1.1s. A refused connection cannot hang, so the request never reaches the network stack. What is left is the detached backend itself, and no instrument outside the process narrows it further. Landing, honestly: - transport runs Linux/X64 + Windows/X64. The macOS cell is EXCLUDED, not xfail-quarantined -- an xfail would let a green gate imply macOS coverage, which is the exact cheat plan_RELEASE SS8.1 refuses. - macOS integration runs "integration and not transport" and prints the gap. - a new required check, transport-known-gaps, names what the gate does NOT verify in the check list itself, so a gap costs a visible line instead of living in a YAML comment nobody reads at review time. What is deliberately NOT claimed: that real Macs are affected. Every data point is from hosted runners, which differ from a real Mac in ways that could matter. finding SS7 gives the one command that settles it. Until someone runs it, F-773 stays open and macOS navigation stays unclaimed in both directions. Also corrects plan_RELEASE_FIX_C in place: its premise was disproved by CI. The original reasoning is left intact -- the evidence was strong and still wrong, which is the part worth keeping. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AminDhouib
added a commit
that referenced
this pull request
Jul 29, 2026
…n (F-780) #43, #45 and #47 are red on the legacy `Browser Integration Tests (Chrome + Xvfb)` check while main and #42 are green on the same check. Read at face value that says FIX-B broke browser integration. It did not, and the reason is provable without reading a single CI log. test.yml runs `pytest -m integration --timeout=120`. The harness it drives declares INIT=60, LIST=130, SPAWN=120, WARMUP=150 with 4 attempts -- under a comment saying "the pytest --timeout is the outer net". Two of those single-step bounds are LARGER than the whole job's per-test budget, and BACKEND_READY_TIMEOUT is exactly equal to it. A 120s outer net cannot contain a 150s inner step. The job is structurally incapable of finishing that test; no timing luck is involved. It only started failing at FIX-B because W1 landed the transport journey already xfail'd (B1 was open), and an xfail costs nothing on the clock. FIX-B C2 correctly removed the marker once B1 was fixed, so from that commit the test actually runs -- and immediately hits the wall. The controlled comparison is #46: it contains FIX-B transitively, runs the SAME `-m integration` selection on the SAME OS under the release gate's 180s budget, and is green 23/23. Deliberately NOT fixed. W2 deletes this job, so patching a doomed file on three in-review branches is churn that also mutates PRs under human review. Recorded instead, because the trap is expensive: the merge queue will show red at #43 -> #45 -> #47 until W2 lands, and someone will otherwise go hunting for a regression that was never there. Not added to RELEASE_CONTRACT.md on purpose -- the contract describes what a user receives, not our merge plumbing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RELEASE-FIX-C — stacked on #44 (audit/release-2-w2)
Fixes F-772, the release-blocking defect W2's macOS cells exposed over 8 CI rounds: no URL could be loaded on macOS over the real user path (stdio proxy -> detached backend). Not the local fixture, not anything.
The mechanism, established by probe rather than inference
setup_interceptionruns on every spawn and, when an instance has zero hooks (the default for anyone who never creates one), enabled a catch-allFetchpattern pair. Chrome then pauses every request before it leaves the browser, and only theFetch.RequestPausedhandler resumes it. On macOS under the detached backend that handler never fires — no_on_request_pausedline appears in any failing backend log — so every navigation hung to the 35s deadline.Decisive evidence (one live instance, same session):
about:blankhttp://127.0.0.1:1/(nothing listening)about:blankclears the tab, the CDP session and cold start. The refused-port row is the proof: a connection to a closed port gets an immediateECONNREFUSEDand cannot hang — so the request never reached the network stack. Corroborated by the fixture server logging 0 hits on every failing run.The fix (c58f51c) — two halves that cannot ship separately
create_hook.create_hooknever calledsetup_interception; interception was only ever established at spawn, so the catch-all was accidentally covering hooks created later. A bare early return would silently have brokenspawn -> create_dynamic_hook -> navigatewith no existing test noticing. Re-arm goes through the samesetup_interception(one home), using a tab cached at setup — so noembeddedmodule importsserver. A_paused_handlersguard keeps re-arming from stacking duplicate handlers, which would double-continue every request.Pins (d5348d6) — RED-first, verified by the orchestrator, not just claimed
test_no_hooks_means_no_fetch_enable— RED pre-fix:assert ['enable'] == [].test_hook_created_after_spawn_arms_interception— RED pre-fix, and RED again when I disabled only the re-arm loop (assert [] == ['enable']), proving that half is load-bearing rather than decorative.test_hooks_still_intercept— green before and after: the feature is not disabled, and patterns/handler count are asserted from real CDP frame arguments.Bonus: every platform gets faster
Linux and Windows enabled catch-all interception too — they merely won the race. With no hooks configured they no longer pay a pause + CDP round-trip per request.
Gates
unit 761 passed (758 + 3 new) - integration 55 passed - ruff clean - ty 76 (exact baseline) - vulture / budgets / suppression-owners clean - no LOC cap padded -
--no-verifynever used.Scope limit — please do not read this as more than it is
This restores the default (zero-hook) path on macOS. It does not fix the handler. Why
Fetch.RequestPausednever dispatches in the detached backend on macOS is still unknown and stays open as F-773; a macOS user who creates a hook re-enables interception and, on current evidence, will hang again. "Navigation works on macOS" is therefore a claim about the default configuration only.Acceptance evidence is #44's macOS cells going green — that CI run, not this description. Merge order: after #42 -> #43 -> #44. True merge commits, please.