fix(extension): inbound-reconnect backoff + page-fetch timeouts (D8-F4 + F6) - #88
Merged
Merged
Conversation
…4, D8-F6)
F4 (offscreen.ts) — the inbound mediator listener re-armed with a single
2s setTimeout from onClose only, and startInbound swallowed errors without
rescheduling. A mediator outage longer than one retry left the listener
dead, silently missing every consent request until the MV3 worker happened
to reboot. Replace with a per-VTA exponential-backoff scheduler
(scheduleInboundReconnect): 2s doubling to 60s, re-arming on FAILED
attempts too, resetting on a successful (re)connect. startInbound now
returns success; reconcileInbound schedules a retry for any VTA that fails
to come up and clears backoff for one that succeeds. onClose fires only on
an unexpected drop, and the forgotten-VTA close path cancels any pending
retry, so a deliberately-closed listener isn't resurrected (a
cancellation guard also covers the in-flight-attempt race). (R1.5)
F6 (background.ts) — the page-facing authenticated fetch proxy
(handleApiGet/handleApiPost) called fetch() with no AbortSignal, so a
blackholed VTA hung the page's await for minutes and stacked requests
pinned the MV3 worker. Route both through a shared proxyFetch that applies
AbortSignal.timeout (20s); a stall now surfaces as a clean
{ ok: false, error } via the dispatcher's existing catch. (R1.2, R4.1
one shared helper rather than two copies)
Type-checks clean (tsc -b packages/extension). No version bump: the
extension is an unpublished artifact and recent fix(extension) PRs
(#82/#86) followed the same convention.
Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
11 tasks
stormer78
added a commit
that referenced
this pull request
Jul 19, 2026
* fix: bound every outbound fetch with a timeout (R1.2) CLAUDE.md names `handleApiGet`/`handleApiPost` as the unbounded fetch that hangs a page against a blackholed VTA. That one was fixed in #88 and is now the one call site in the repo that complies. Nine others never were. They hid from a naive audit: a literal `grep "fetch("` finds a single call site repo-wide. This package injects `fetch` for testability — const f = opts.fetch ?? fetch.bind(globalThis); — so the real calls are spelled `f(...)`, `fetchFn(...)`, `this.fetchImpl(...)`, and none of the six files containing them mentioned `signal`, `AbortSignal` or `timeout`. An unbounded fetch against a wedged peer does not fail slowly; it does not fail at all. The page waits on a promise that never settles, and in MV3 the unresolved `await` also pins the service worker awake and stacks later requests behind it. Nothing surfaces an error, so nothing reports it. Adds `withFetchTimeout`, applied at each INJECTION point rather than each call site — that is what makes the coverage complete, and it matters most for `getVtaBearer`, which runs before every REST request and is reached only indirectly through `VtaAuthInputs`. Leaving it unbounded would have defeated a timeout added anywhere downstream. A caller-supplied `signal` is combined with the deadline rather than replaced: silently disabling a caller's cancellation would be a worse bug than the one being fixed. Where `AbortSignal.any` is unavailable the timeout survives, on the principle that losing the combination is recoverable and losing the deadline is the defect. Adds `e.client.timeout` so "the VTA never answered" is distinguishable from "the connection failed" by a stable code rather than message text (R3.7). `isFetchTimeout` matches `DOMException.name`, the platform's own machine-readable discriminant. Sites bounded: vault/transport.ts (x2, the bearer handshake), vta/rest-channel, vta/client, rp-login/step-up (x2), siop/login-client (x2), device/register-gateway. Verified: 10 new tests — deadline fires on a blackholed peer, fast responses untouched, caller's abort still wins, timeout survives a non-aborting caller signal, init fields preserved, global-fetch fallback bounded, plus a wiring test asserting a real exported call path (`registerPushChannel`) hands a signal to its injected fetch. Mutation-tested: with the timeout removed the suite HANGS rather than failing, which is precisely the production symptom. Full suite 190/190, clean build and lint. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> * test: hold the event loop open while a timeout test awaits its deadline The new timeout tests passed locally and failed on CI with every test in the file reported as "Promise resolution is still pending but the event loop has already resolved". Node unrefs the timer behind `AbortSignal.timeout`, so a pending abort does not hold the event loop open. The blackhole stub's promise had nothing else queued, so on CI the process drained and exited before the deadline fired. Locally it survived only because other work happened to keep the loop alive — the tests were depending on an accident. A ref'd interval, cleared when the abort lands, removes the dependency. Signed-off-by: Glenn Gore <glenn.g@affinidi.com> --------- Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
stormer78
added a commit
that referenced
this pull request
Jul 19, 2026
CLAUDE.md was untracked, so every contributor had their own copy or none. Checks it in, and corrects the parts that had gone stale — its named defects were the ones most likely to be trusted verbatim, and all three were fixed: - R3.7's `consentRequiredFrom` example was fixed (it now matches `details.reason`). Replaced with the rule that actually bites now: a Response body reads once, so an already-parsed body must use `errorFromBody`, not a re-read of the spent Response (#99). - R1.5's "one 2s retry from onClose" applied to the worker inbound path (fixed in #88) and then to the approver inbox (fixed in #97). Replaced with the invariant — cap the delay not the attempt count, re-arm on every failure including first-connect — and a pointer to `ReconnectScheduler`. - R1.2's `handleApiGet`/`handleApiPost` example was fixed in #88 and is now the compliant reference. Replaced with the thing that actually hides these: fetch is injected, so `grep "fetch("` finds almost nothing and the timeout belongs at the injection point (#98). Promotes the one genuinely open defect to its own section: R1.6 persist-before-ack, which is not fixable from this repo — vti-didcomm-js acks before dispatching to `onMessage`, and the wallet persists only the message id, so an offscreen teardown mid-prompt loses a task-consent request for good. Adds a repo-mechanics section for the traps that cost time this week: build `core` before typechecking dependents, lint is `tsc -b` (never `-b --noEmit`, TS6310), cross-workspace imports need a `references` entry, what CI asserts, stub with real `Response` objects, and Node unreffing the `AbortSignal.timeout` timer (passes locally, fails in CI). Signed-off-by: Glenn Gore <glenn.g@affinidi.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.
Two independent MV3 network-resilience fixes in the extension package (D8-F4,
D8-F6). Both are cases where a mediator/VTA outage silently degrades the wallet.
F4 — inbound listener survives a mediator outage (
offscreen.ts, R1.5)The inbox mediator session re-armed with a single 2s
setTimeoutfromonClose, andstartInboundswallowed errors without rescheduling. So amediator outage longer than that one retry left the inbound listener dead —
silently missing every consent/confirm request — until the MV3 worker happened
to reboot. Since consent prompts are security controls, a missed one is a gated
action that never got its human check.
scheduleInboundReconnect(vtaDid): per-VTA exponential backoff (2sdoubling to 60s), idempotent per VTA (no stacked timers), re-arming on
failed attempts too, resetting to base on a successful (re)connect.
startInboundnow returns success;reconcileInboundschedules a retry forany VTA that fails to come up and clears backoff for one that succeeds.
onClosefires only on an unexpected drop (not our ownclose()), and theforgotten-VTA close path calls
clearInboundBackoff— so a deliberately-closedlistener is never resurrected. A cancellation guard also covers the race where
a VTA is forgotten while a retry's
startInboundis in flight.F6 — page-facing fetch can't hang the page (
background.ts, R1.2)handleApiGet/handleApiPost(the authenticated fetch the wallet proxies fora page, using its host permissions) called
fetch()with noAbortSignal,so a blackholed VTA hung the page's
awaitfor minutes and stacked requestspinned the MV3 worker awake.
proxyFetchapplyingAbortSignal.timeout(20s). A stall surfaces as a clean{ ok: false, error }through the message dispatcher's existing
.catch(with a descriptive"VTA unreachable or not responding" message on the timeout path).
Verification
tsc -b packages/extensiontype-checks clean. (This repo has no CIworkflows; the two pre-existing
tsc -b --noEmitproject-reference / workspaceerrors are unrelated to these files and reproduce on clean
main.)fix(extension)PRs (fix(extension): take the origin from the browser, not from the message body #82, fix(extension): the relay's origin was body-supplied; signTrustTask could self-approve #86).Completes the locally-actionable part of D8. F5 (pnm-relay) remains — that repo
isn't checked out here.