The outage skip I added this morning was too generous in three ways (#210)

An adversarial review found three paths by which a genuine report defect could take the skip I added
in #206. All three reproduce by reading the code, and all three are mine.

**Uncaught JS was swallowed.** `unreachable_external_host` looked only at `console_errors`, and the
callers skip *before* asserting `page_errors`. So a report throwing an uncaught exception was skipped
whenever its console errors happened to be outage-shaped — the single worst outcome for a guard whose
entire job is deciding whether CI may fail. A remote host refusing a connection cannot cause an
uncaught exception in page code, so one is now disqualifying on its own. The assertion also moved
*before* the skip: redundant now, but the ordering was the root cause and is what a future edit is
most likely to get wrong again.

**Any failing external URL counted.** The old version required only that *some* external request had
been attempted — and since all 19 reports contact UCSC, that was satisfied essentially always. One
unrelated refused request therefore disabled every verdict for that report: blank-canvas,
`_MIN_CANVASES`, convergence and the load budget. The URLs implicated in the failures must now all
resolve to a reference-sequence host.

**A wrong host committed inside a report looked like weather.** A typo'd or third-party host baked
into a report is a permanent defect for every reader, and it produced the same console error as an
outage — so it would have been skipped forever. Restricting the skip to `REFERENCE_SEQUENCE_HOSTS`
makes a report citing anything else fail.

My first attempt at that last fix was itself wrong, and this file's own new test caught it: I compared
with `host in url`, which also matches `hgdownload.soe.ucsc.edu.example.net` — letting a hijacked
domain inherit the trusted host's exemption. Now an exact netloc comparison via `urlparse`.

One case stays genuinely undecidable and is documented rather than papered over: a *correct* reference
URL that is permanently dead is indistinguishable from a transient outage in a single render. It
skips, and the docstring says where to look if UCSC ever retires that path.

5 new tests, one per hole plus the message accuracy and the case the skip exists for. Browser suite
46 passed with UCSC reachable; fast suite 2,013 passed / 30 skipped.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: lucapinello

tests/browser_harness.py

@@ -142,33 +142,83 @@ def summary(self) -> str:
 )
 
 
+#: The only host a committed report is *expected* to fetch from: igv.js needs a sequence source and
+#: hg38 is ~3 GB, so the reference two-bit is resolved from UCSC at open time. Anything else failing
+#: is a defect in the report, not weather — so it must not qualify for a skip.
+#:
+#: Compared as an **exact netloc**, never as a substring. `"hgdownload.soe.ucsc.edu" in url` also
+#: matches `hgdownload.soe.ucsc.edu.example.net`, which would let a typo'd or hijacked domain inherit
+#: the trusted host's exemption. That was the first implementation, and this file's own test caught it.
+REFERENCE_SEQUENCE_HOSTS = frozenset({"hgdownload.soe.ucsc.edu"})
+
+
+def _urls_in(message: str) -> list:
+    """URLs named inside a console error, e.g. igv.js's "Error accessing resource: <url> Status: 0"."""
+    import re
+
+    return re.findall(r"https?://[^\s\"'<>]+", message)
+
+
 def unreachable_external_host(r) -> "str | None":
     """Why this render cannot be judged, or None if it can.
 
     Every committed report resolves its reference sequence from `hgdownload.soe.ucsc.edu`: igv.js
     requires a sequence source, hg38 is ~3 GB, and bundling it in a repo is not an option — a
-    documented limitation rather than a defect. The consequence is that this file's verdict depended
-    on a third party being up. On 2026-08-14 UCSC refused the connection during one PR's run and the
+    documented limitation rather than a defect. The consequence was that this file's verdict depended
+    on a third party being up: on 2026-08-14 UCSC refused the connection during one PR's run and the
     same corpus that had passed minutes earlier reported `canvases 0/0 painted (NOT converged)` and
     blew the 30 s budget at the 60 s timeout — four failures, none of them about the reports.
 
-    A test that goes red when someone else's server hiccups trains people to re-run CI until it is
-    green, which is how a real blank-panel regression gets waved through. So: if the *only* console
-    errors are unreachable-host signatures and an external host was actually contacted, this render
-    is unverifiable and says so. A genuinely broken report still fails — a blank canvas with UCSC up
-    produces no such console error, and any error outside this list disqualifies the skip.
+    A suite that goes red when someone else's server hiccups trains people to re-run CI until it is
+    green, which is how a real blank-panel regression gets waved through. But the first version of
+    this function was far too generous, and an adversarial review found three ways a genuine defect
+    could take the skip. All three are now closed:
+
+    * **Uncaught JS was swallowed.** The old version looked only at `console_errors`, and the callers
+      skip *before* asserting `page_errors` — so a report throwing an uncaught exception was skipped
+      whenever the console errors happened to be outage-shaped. An uncaught error is now
+      disqualifying on its own: it cannot be caused by a remote host refusing a connection.
+    * **Any failing external URL counted.** The old version required only that *some* external
+      request had been attempted. Since all 19 reports contact UCSC, that was satisfied
+      essentially always, so one unrelated refused request disabled every verdict for that report.
+      Now the URLs named in the failures must all be reference-sequence URLs.
+    * **A wrong URL committed inside a report looked like weather.** A typo'd or third-party host
+      baked into a report is a permanent defect for every reader, and it produced the same console
+      error as an outage. Restricting the skip to `REFERENCE_SEQUENCE_HOSTS` means a report citing
+      anything else fails instead of being skipped forever.
+
+    One case remains genuinely undecidable and is called out rather than papered over: a *correct*
+    reference URL that is permanently dead looks exactly like a transient outage. Nothing observable
+    in one render distinguishes them, so it skips. If UCSC ever retires that path, every report will
+    skip rather than fail, and this docstring is the note that says to look here.
     """
     if not r.console_errors or not r.external_urls:
         return None
+
+    # An uncaught JS exception is never explained by an unreachable host.
+    if getattr(r, "page_errors", None):
+        return None
+
     unexplained = [e for e in r.console_errors
                    if not any(sig in e for sig in UNREACHABLE_SIGNATURES)]
     if unexplained:
         return None
-    hosts = ", ".join(r.external_hosts) or "an external host"
+
+    # Every URL implicated in the failures must be one we expect a report to fetch. When a message
+    # names no URL (Chromium's bare "Failed to load resource: net::ERR_*"), fall back to requiring
+    # that every external request this report made was a reference-sequence one.
+    named = [u for e in r.console_errors for u in _urls_in(e)]
+    implicated = named or list(r.external_urls)
+    hosts = {urlparse(u).netloc for u in implicated}
+    foreign = sorted(h for h in hosts if h not in REFERENCE_SEQUENCE_HOSTS)
+    if foreign:
+        return None
+
+    where = ", ".join(sorted(hosts)) or "the reference host"
     return (
-        f"{r.path.name}: {hosts} did not answer, so the reference sequence never loaded and the "
-        f"panel could not paint. Not a report defect -- igv.js needs a sequence source and hg38 is "
-        f"too large to bundle. Errors: {r.console_errors[:2]}"
+        f"{r.path.name}: {where} did not answer, so the reference sequence could not load. Not a "
+        f"report defect -- igv.js needs a sequence source and hg38 is too large to bundle. "
+        f"Errors: {r.console_errors[:2]}"
     )
 
 

tests/test_browser_outage_guard.py

@@ -68,3 +68,69 @@ def test_errors_without_any_external_request_are_never_an_outage():
 
 def test_a_clean_render_needs_no_excuse():
     assert bh.unreachable_external_host(_fake()) is None
+
+
+# ── the three holes an adversarial review found in the first version ─────────────
+
+def test_an_uncaught_js_exception_is_never_excused_by_an_outage():
+    """The worst of the three: callers skip BEFORE asserting page_errors.
+
+    A report throwing an uncaught exception was skipped whenever its console errors happened to be
+    outage-shaped — so a genuine JS fault in a committed report could hide behind UCSC being down.
+    A remote host refusing a connection cannot cause an uncaught exception in page code.
+    """
+    r = _fake(console=_REAL_OUTAGE, external=_UCSC,
+              page=["TypeError: Cannot read properties of undefined (reading 'tracks')"])
+    assert bh.unreachable_external_host(r) is None, (
+        "a render with uncaught JS was treated as an outage; the paint test skips before it asserts "
+        "page_errors, so that fault would never be reported"
+    )
+
+
+def test_a_failing_third_party_url_is_not_an_outage_excuse():
+    """The skip once required only that *some* external request had been attempted.
+
+    All 19 reports contact UCSC, so that condition was satisfied essentially always — meaning one
+    unrelated refused request disabled every verdict for the report, including blank-canvas.
+    """
+    r = _fake(
+        console=["error: IGV error: Error accessing resource: "
+                 "https://cdn.example.com/tracks/mystery.bw Status: 0"],
+        external=["https://cdn.example.com/tracks/mystery.bw"] + _UCSC,
+    )
+    assert bh.unreachable_external_host(r) is None, (
+        "a report failing to fetch a non-reference third-party URL was skipped. That is a defect in "
+        "the report — it should not ship a dependency on cdn.example.com — and must fail, not skip."
+    )
+
+
+def test_a_typoed_reference_host_committed_in_a_report_still_fails():
+    """A wrong host baked into a report is permanent for every reader, not weather."""
+    bad = "https://hgdownload.soe.ucsc.edu.example.net/goldenPath/hg38/bigZips/hg38.2bit"
+    r = _fake(console=[f"error: IGV error: Error accessing resource: {bad} Status: 0"], external=[bad])
+    why = bh.unreachable_external_host(r)
+    assert why is None, (
+        f"a report citing {bad} was skipped as an outage. A typo'd or hijacked host is a permanent "
+        f"defect for every reader and must fail. (got: {why})"
+    )
+
+
+def test_the_real_reference_url_still_skips():
+    """The whole point: the genuine UCSC outage must still be tolerated."""
+    r = _fake(console=_REAL_OUTAGE, external=_UCSC)
+    why = bh.unreachable_external_host(r)
+    assert why and "hgdownload.soe.ucsc.edu" in why, (
+        f"tightening the guard broke the case it exists for: {why!r}"
+    )
+
+
+def test_the_skip_message_names_only_hosts_that_actually_failed():
+    """The old message listed every contacted host and asserted the sequence never loaded.
+
+    With the URL now extracted from the error itself, the message can only name what failed.
+    """
+    r = _fake(console=_REAL_OUTAGE, external=_UCSC + ["https://fonts.googleapis.com/css"])
+    why = bh.unreachable_external_host(r) or ""
+    assert "fonts.googleapis.com" not in why, (
+        f"the skip message blames a host that did not fail: {why!r}"
+    )

tests/test_committed_reports_render_in_a_browser.py

@@ -129,11 +129,15 @@ def test_every_committed_report_paints_every_track(browser, report):
     """
     r = _render_once(browser, report)
 
+    # Asserted BEFORE the outage skip, deliberately. `unreachable_external_host` also refuses to
+    # excuse a render with uncaught JS, so this is belt and braces — but the original defect was
+    # precisely that the skip ran first and swallowed this assertion, and the ordering is the part a
+    # future edit is most likely to get wrong again.
+    assert not r.page_errors, f"{r.summary()}\nuncaught JS: {r.page_errors[:3]}"
+
     unverifiable = bh.unreachable_external_host(r)
     if unverifiable:
         pytest.skip(unverifiable)
-
-    assert not r.page_errors, f"{r.summary()}\nuncaught JS: {r.page_errors[:3]}"
     assert not r.console_errors, f"{r.summary()}\nconsole errors: {r.console_errors[:3]}"
 
     if not r.is_igv: