Skip to content

Fix E2E on Blacksmith runners: DoH polling with cache purge + safer PR trigger - #45

Merged
ycmjason merged 15 commits into
mainfrom
claude/e2e-blacksmith-runner-config-99e4jp
Jul 14, 2026
Merged

Fix E2E on Blacksmith runners: DoH polling with cache purge + safer PR trigger#45
ycmjason merged 15 commits into
mainfrom
claude/e2e-blacksmith-runner-config-99e4jp

Conversation

@ycmjason

@ycmjason ycmjason commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

E2E has been failing since the workflow moved to Blacksmith runners (#41). This PR makes the suite runner-proof and keeps it on Blacksmith, and fixes a security/correctness issue in how the workflow runs on PRs.

Root cause

Two separate DNS problems:

  1. Blacksmith's system resolver never surfaced freshly created challenge TXT records within the 10-minute polling budget, failing every test.
  2. After switching polling to DNS-over-HTTPS, a subtler race remained: polling starts the instant the Cloudflare API returns, so the first lookup can arrive before the record reaches Cloudflare's authoritative edge — the resulting NXDOMAIN gets negatively cached by the resolver for the zone's negative TTL (~30 min), outliving the polling budget. Observed twice (runs 78, 86) as one domain stuck at [] for the full 10 minutes while sibling records resolved in seconds.

Changes

  • e2e/utils/resolveDns.ts (new): e2e DNS polling now purges the record from 1.1.1.1's cache (via the endpoint behind https://one.one.one.one/purge-cache/) before every lookup, then resolves over Cloudflare DoH. This bypasses the runner's DNS stack entirely and defeats the negative-cache race. Note: the purge endpoint is unofficial — if polling ever starts timing out with ⚠️ Failed to purge warnings in the log, check that endpoint first.
  • e2e tests: import the new resolver instead of resolveDns.deno (system resolver).
  • e2e.yaml:
    • runs-on: blacksmith-2vcpu-ubuntu-2404 (kept on Blacksmith).
    • Replaced pull_request_target + e2e:approve label with a plain pull_request trigger and the protected e2e environment. The old setup ran the workflow and code from the base branch, so PR runs never tested PR changes; the new setup tests the PR's own code, while fork PRs only receive the Cloudflare secrets after an explicit "Approve and run" (repo is set to require approval for all outside collaborators).

src/ is untouched — no published package behavior changes.

Verification

Bare Cloudflare DoH (no purge) failed e2e runs 78 and 86 with the stuck-record signature; purge variants passed runs 82–85 and 87, typically completing in ~3 minutes on Blacksmith.

https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9

&lt;img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"&gt; <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1786626697&amp;installation_id=142134082&amp;pr_number=45&amp;repository=fishballapp%2Facme&amp;return_to=https%3A%2F%2Fgithub.com%2Ffishballapp%2Facme%2Fpull%2F45&amp;signature=f6b8bb178033d246c929346d4b6c5cad69138055d213d5426da1a80ed3a3d724" rel="nofollow noreferrer noopener" target="_blank">&lt;img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"&gt;
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

@claude
fix(e2e): poll DNS over DoH so runner DNS caching can't stall challenges
adf34ac 
Since the E2E workflow moved to Blacksmith runners (ec9fad2), all three
e2e tests time out in pollDnsTxtRecord: the Cloudflare TXT records are
created successfully, but the runner's system resolver keeps returning
empty results for the full 10-minute polling window.

Blacksmith routes DNS through its own resolver infrastructure, so
freshly created records can stay invisible to Deno.resolveDns far longer
than on GitHub-hosted runners. Instead of depending on the runner's DNS
path, the e2e tests now poll propagation via the package's own DoH
resolver (Cloudflare endpoint), which rides plain HTTPS and behaves the
same on any runner.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@ycmjason ycmjason added the e2e:approve approve running e2e label Jul 14, 2026
claude added 2 commits July 14, 2026 13:13
@claude
Revert "fix(e2e): poll DNS over DoH so runner DNS caching can't stall…
fd600b5 
… challenges"

This reverts commit adf34ac.
@claude
ci(e2e): switch E2E workflow back to GitHub-hosted runner
4df3d9c 
The E2E suite has been failing on Blacksmith runners because their DNS
path never surfaces the freshly created challenge TXT records to the
test's polling. Move back to ubuntu-latest where the suite passes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@ycmjason ycmjason added e2e:approve approve running e2e and removed e2e:approve approve running e2e labels Jul 14, 2026
@claude
ci(e2e): gate PR e2e runs with a protected environment instead of pul…
9468134 
…l_request_target

pull_request_target ran the workflow and checked out the code from the
base branch, so labeled PR runs only ever re-tested main — a PR could
never actually exercise its own changes.

Switch to a plain pull_request trigger with a protected 'e2e'
environment: runs now use the PR's own workflow and code, while secret
access still requires a manual approval on every run, including new
pushes and fork PRs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
claude added 2 commits July 14, 2026 13:24
@claude
Reapply "fix(e2e): poll DNS over DoH so runner DNS caching can't stal…
2aa560f 
…l challenges"

This reverts commit fd600b5.
@claude
ci(e2e): trial Blacksmith runner again, now with DoH-based DNS polling
27c428f 
With PR e2e runs now testing PR code, this combination can be verified
for real: the DoH resolver should sidestep the Blacksmith DNS issue that
broke polling before.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
fix(e2e): timeout + fallback for DoH polling so stalled requests can'…
5214282 
…t hang the suite

The Blacksmith trial hung mid-run: the DoH fetch had no timeout, so a
silently stalled connection to the resolver endpoint blocked
pollDnsTxtRecord forever — its polling budget is only checked between
attempts.

Add an optional timeout to createResolveDns (resolveDns.doh) via
AbortSignal.timeout, and make the e2e resolver try Cloudflare then
Google with a 10s per-request timeout, logging failures and returning
'not visible yet' so polling retries. A full DoH outage now surfaces as
the regular polling TimeoutError with diagnostics in the log.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
fix(e2e): merge answers from multiple DoH resolvers + propagation gra…
f6b50be 
…ce period

Run 78 showed the real failure mode: DoH works on Blacksmith (2 of 3
tests passed), but polling starts the instant the Cloudflare API
returns, so the first query can hit the resolver edge before the record
propagates — the resulting NXDOMAIN gets negative-cached longer than the
10-minute polling budget, leaving one unlucky domain stuck at [].

Two mitigations:
- resolveDns now queries Cloudflare and Google DoH in parallel and
  merges answers, so a single resolver's stale negative cache can't
  stall polling; failures are logged and treated as 'not visible yet'.
- Tests wait 15s after creating DNS records before the first lookup, so
  the negative cache is far less likely to be seeded at all.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
test(e2e): drop DNS propagation grace period
d408f30 
Trial whether the merged multi-resolver DoH polling alone is enough to
keep e2e green on Blacksmith, without the extra 15s wait after record
creation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
fix(e2e): purge 1.1.1.1 cache on empty DNS answers instead of merging…
b82a9a5 
… Google DoH

Simplify the e2e resolver back to Cloudflare DoH only. When a lookup
returns empty, flush the record from 1.1.1.1 via the purge endpoint
behind one.one.one.one/purge-cache, so a negatively cached NXDOMAIN
seeded by a lookup that raced record propagation can't outlive the
polling budget — the next attempt re-queries the authoritative servers.

The purge endpoint is unofficial; lookup and purge failures are logged
and treated as 'record not visible yet' so polling keeps retrying.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
docs(ci): update e2e environment comment to match unprotected-env setup
de9d787 
The environment no longer has required reviewers; fork runs are gated by
the repo-level 'require approval for all outside collaborators' setting
instead. Describe what the environment actually does now.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
refactor: drop DoH timeout option, raise default DNS poll interval to…
67617a5 
… 10s

The timeout option never addressed an observed failure — remove it to
keep the resolveDns.doh API surface unchanged. Keep a hardcoded abort on
the (unofficial) 1.1.1.1 purge request only.

Bump pollDnsTxtRecord's default interval from 5s to 10s: DNS propagation
is slow enough that 5s polling mostly burns queries (and now purge
requests) without finding records sooner.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
refactor(e2e): strip DNS polling to the minimal DoH resolver
a07b02c 
Remove the 1.1.1.1 purge-on-empty logic and restore the 5s default poll
interval, leaving only the Cloudflare DoH resolver — the minimal change
over the system resolver — to find out how much machinery the Blacksmith
fix actually needs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@claude
fix(e2e): purge 1.1.1.1 cache before every DNS lookup
9e42c31 
Bare Cloudflare DoH polling failed twice (runs 78, 86) with a TXT record
stuck at [] for the whole 10-minute budget — an early lookup racing
record propagation gets its NXDOMAIN negatively cached for longer than
we can afford to wait. Purge before each lookup so every poll attempt
effectively re-queries the authoritative servers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JJeFA4CinTHKcfKHwCE7a9
@ycmjason ycmjason changed the title Use DoH for DNS resolution in e2e tests Fix E2E on Blacksmith runners: DoH polling with cache purge + safer PR trigger Jul 14, 2026
@ycmjason ycmjason removed the e2e:approve approve running e2e label Jul 14, 2026 — with Claude
@ycmjason
ycmjason merged commit 71c94ff into main Jul 14, 2026
6 checks passed
@ycmjason
ycmjason deleted the claude/e2e-blacksmith-runner-config-99e4jp branch July 14, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ycmjason @claude