Abort and retry a base-realm fetch that stalls before response headers

[habdelra]

Host acceptance shards intermittently hang on a base-realm fetch — most visibly the Spec preview tests — when a request to the base realm's _info (or another base artifact) is issued but its response headers never arrive. The fetch neither resolves nor rejects, so the fetcher test-waiter never settles, settled() blocks, and the test burns all the way to QUnit's global timeout. Because the wedged request is still in flight when the timeout fires, teardown pulls the mock realm out from under the next test or two, so they cascade with TypeError: Failed to fetch.

The virtual network already carries a retry net for the closely-related case where base-realm artifact fetches "vanish" with a thrown TypeError: Failed to fetch. That net only engages on a thrown error, so the stall variant — headers that never come — slips past it: there is nothing to catch and no bound on the wait.

Fetch handling — packages/runtime-common/virtual-network.ts

• Bound how long a retryable base-realm fetch may wait for response headers, and abort past that bound. The abort turns the stall into the same retryable failure the net already recovers from — on a fresh connection the retry gets its headers.
• Each attempt uses its own AbortController, so a stalled attempt's abort never poisons the next. The timer is cleared the instant the fetch settles, so a resolved streaming response is never aborted — withRetries only awaits the response's headers, not its body.
• The bound is scoped to the browser test environment and to already-retryable URLs, so a legitimately slow response in node / worker / env-mode (e.g. a heavy _search) is never aborted and retried. Production fetch behavior is unchanged.
• buildRequest carries the abort signal across the virtual-to-real URL remap, so a base artifact addressed at the virtual https://cardstack.com/base/... host still sees the abort once the host maps it to the resolved realm URL.
• The header timeout is injectable on VirtualNetwork (default 10s) so the unit tests can drive the stall → abort → retry → success path deterministically.
• The per-attempt retry log now includes the error, so a failure shows which shape it took — a thrown TypeError: Failed to fetch or an aborted header stall (FetchHeaderTimeout).

Tests pin which URLs the bound covers, that the signal survives the URL remap, and the end-to-end recovery.

Unrelated fixup — packages/software-factory

qunit-testing.md now ships in the built boxel skill, so it no longer belongs in PENDING_BOXEL_REFERENCES — the skill-loader validation test fails while a shipped reference is still listed as pending. The list is emptied, and the "pending references are still pending" test accepts the empty case via expect().

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dc026b868d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files      1 suites   3h 4m 28s ⏱️
3 658 tests 3 643 ✅ 15 💤 0 ❌
3 677 runs  3 662 ✅ 15 💤 0 ❌

Results for commit 6d08539.

Realm Server Test Results

    1 files      1 suites   13m 40s ⏱️
1 934 tests 1 934 ✅ 0 💤 0 ❌
2 013 runs  2 013 ✅ 0 💤 0 ❌

Results for commit 6d08539.

[Copilot]

Pull request overview

This PR prevents browser-based host tests from hanging indefinitely when a retryable base-realm fetch stalls before response headers arrive, by adding a per-attempt header timeout that aborts and retries the request (test environment only) and by ensuring abort signals survive virtual-to-real URL remapping.

Changes:

• Extend VirtualNetwork/withRetries to optionally apply a per-attempt “response headers must arrive by” timeout (gated to browser + __environment === 'test' + already-retryable URLs), aborting and retrying stalled attempts.
• Ensure per-attempt abort signals are attached to the underlying native fetch even when the request URL is remapped (buildRequest now preserves signal; runFetch merges caller + per-attempt signals).
• Add unit tests that pin the timeout gating and the stall → abort → retry → success path, plus signal propagation through buildRequest.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
packages/runtime-common/virtual-network.ts	Adds injectable header-timeout plumbing + per-attempt abort signals in retry loop; preserves abort signals through request remapping.
packages/host/tests/unit/virtual-network-retry-test.ts	Adds unit coverage for timeout gating, header-stall recovery, and remapped-request signal propagation.

Comments suppressed due to low confidence (4)

packages/runtime-common/virtual-network.ts:743

• [Claude Code 🤖] The retry log line reads a bit ungrammatically ("Encountered fetch failed for …"). Since this is newly edited logging, it’s a good spot to tighten the message for easier grepping and readability.

        `Encountered fetch failed for ${url.href} (${err?.name ?? 'Error'}: ${

packages/host/tests/unit/virtual-network-retry-test.ts:123

• [Claude Code 🤖] The new assertion message mentions a CI-specific hang. Since the predicate is a general contract (what gets bounded), the message can be phrased without environment-specific history.

        'the base _info fetch that hung in CI is bounded',

packages/host/tests/unit/virtual-network-retry-test.ts:168

• [Claude Code 🤖] These new comments describe a past CI failure ("Mirrors the CI failure…") and use past-tense wording. Rewording them to describe the modeled failure mode keeps the comment evergreen.

// fetch does), then succeeds. Mirrors the CI failure where base/_info headers
// never arrived and the unbounded fetch hung until QUnit's global timeout.

packages/host/tests/unit/virtual-network-retry-test.ts:184

• [Claude Code 🤖] In the stalled-attempt stub, the !signal branch returns without resolving or rejecting, which can hang the entire unit test suite if the signal wiring regresses. It’s safer to fail fast here so the test fails quickly (and then the later signalSeen() assertion still catches the regression) rather than timing out the whole run.

        if (!signal) {
          return; // no signal wired => hang (fix broken); QUnit will time out
        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[burieberry]

Reviewed the retry/abort changes. The core fix is well-constructed — per-attempt AbortController, header-arrival timeout, clearTimeout on settle so a resolved body stream is never cut, a fresh signal per attempt, and carrying the signal through the virtual→real remap. No concerns with the mechanism itself.

One thing worth fixing before merge:

withRetries retries caller-initiated aborts. The catch gates retry on shouldRetryFetch(url) alone and never inspects the error type, so an AbortError from a caller-supplied signal is treated as a transient failure — retried up to 10× with backoff (~5.5s), re-merging the already-aborted signal each round, before finally rethrowing. Now that buildRequest carries the signal through the remap, this is reachable outside tests too. Suggested guard in the catch:

} catch (err: any) {
  // A caller-initiated abort isn't transient — surface it immediately.
  if (err?.name === 'AbortError') {
    throw err;
  }
  if (!shouldRetryFetch(url) || ++attempt > maxAttempts) { ... }

The per-attempt timeout abort sets err.name = 'FetchHeaderTimeout', so it isn't caught by this guard and the stall-retry path keeps working.

Minor / optional: the AbortSignal.any fallback in mergeAbortSignals returns the last signal (the timeout), silently dropping the caller signal. It's dead code by the comment's own claim, but if you want it correct rather than removed, do a manual merge there.

🤖 Reviewed with Claude Code