fix(cli): recover transient publish failures

[miguel-heygen]

Summary

hyperframes publish now recovers from a transient failure while preparing the staged upload or sending the archive to S3, instead of immediately ending with the opaque fetch failed. The retry waits briefly so a DNS/socket interruption can clear, while request timeouts stay single-shot to avoid doubling the worst-case wait for large archives.

If a request still fails, the CLI names the failed stage, preserves the underlying network code, syscall, errno, and message, redacts presigned URL credentials, and explains how to enable Node's environment-proxy support when proxy variables are present. The proxy note is explicitly conditional so it does not claim the proxy caused an unrelated failure.

The completion request deliberately remains single-shot: the backend consumes and deletes the staged object, so blindly retrying it could duplicate an anonymous project or retry a publish that already committed. HTTP errors also remain non-retryable, and the legacy direct-upload fallback behavior is unchanged.

Root cause

Three different network boundaries were collapsed into the same top-level fetch failed message because the CLI discarded Error.cause. Production checks returned 200 from the HeyGen prepare and complete endpoints and from the presigned S3 PUT, while a forced DNS failure reproduced the reported output exactly. This points to a client transport interruption—not a current backend application error—and makes bounded recovery plus stage-specific diagnostics the appropriate fix.

Test plan

• publishProject.test.ts: 37/37
• Full CLI suite: 2,481 passed, 2 skipped
• CLI typecheck
• Full monorepo build
• oxfmt, oxlint, git diff --check
• Live hyperframes@0.7.98 production publish exercised prepare → S3 PUT → complete successfully
• Forced unreachable API reproduced the old fetch failed path and verified the new stage/cause output

---

[vanceingalls]

R1 adversarial review — fix(cli): recover transient publish failures

Verdict: APPROVE

Design lens

The retry policy is scoped correctly. fetchForPublish catches only exceptions from fetch() itself — in Node's undici that means transport-level failures (DNS, TCP, TLS, AbortError). HTTP 4xx/5xx come back as Response objects and continue to flow through readErrorMessage unchanged, so this PR does not silently retry user errors or backend HTTP errors (per feedback_widened_retry_policy_audit.md — the widened-taxonomy audit passes: transport-only, bounded at 2).

Idempotency is preserved:

• prepare retry re-mints a fresh upload_key (previous one is orphaned; server TTL cleans up) — safe.
• S3 PUT retry hits the same signed URL/key — S3 PUT is idempotent on a stable key — safe.
• complete deliberately kept single-shot (packages/cli/src/utils/publishProject.ts:627-644, attempts=1). PR body cites the "backend consumes and deletes the staged object" reason; this is the correct call — retrying a completed finalize would risk duplicating an anonymous project.
• Legacy direct-publish path (:543-556) also stays single-shot — same reasoning applied uniformly across the "may have server-side committed" surfaces (legacy multi-role endpoint lens: role-appropriate retry policy per stage).

Cancellation chain is clean: createInit factory is invoked per attempt, so each retry gets a fresh AbortSignal.timeout(...). Signals are never reused across attempts. No external AbortController is threaded in, so no user-cancel-mid-retry hazard.

Motivation ↔ fix fit passes (per feedback_ground_truth_first_before_pr_open.md): user's opaque fetch failed is undici's TypeError('fetch failed'); the new tests explicitly cover EAI_AGAIN, ECONNRESET, ENETUNREACH, and the HTTPS_PROXY-without-NODE_USE_ENV_PROXY misconfig — that IS the reported class.

Test hygiene passes (per feedback_test_asserts_guard_semantics_not_presence.md): tests verify behavior, not presence — retry count, URL identity between attempts (mock.calls[1] vs mock.calls[2]), stage-named error text, rejects.not.toThrow("do-not-print") for signed-URL cred leakage, and that finalize does NOT retry (only 1 mockRejectedValueOnce, expects it to bubble).

Secret hygiene passes: redactUrlQuery strips ?[query] from any URL in the error text, and there's a dedicated test asserting the X-Amz-Signature=do-not-print value never appears in the surfaced error.

Nits (non-blocker)

1. No backoff/jitter between attempts (packages/cli/src/utils/publishProject.ts:211-222) — retry is immediate. For transient DNS (EAI_AGAIN), an immediate second query is likely to hit the same failed resolver state. A small jittered delay (100-300ms) would meaningfully raise the recovery rate for the exact class this is designed to handle. Bounded at 2 attempts so the current shape is safe; just leaving performance on the table.

2. Unreachable fallthrough (packages/cli/src/utils/publishProject.ts:223) — throw new Error(failureStage) after the for loop can never execute because attempts >= 1 by construction and the final attempt always either returns or throws. Dead but harmless; TS control-flow needs it for the return type.

3. errorCode scope (packages/cli/src/utils/publishProject.ts:170-173) — only inspects .code. Undici does surface .code on .cause so the common case is covered, but errno and syscall are also standard fields on Node system errors and could be worth appending for messages like ENOTFOUND that come through libc-flavored paths. Minor.

4. proxySupportHint() fires unconditionally (packages/cli/src/utils/publishProject.ts:179-190) — appended to every describeFetchFailure output whenever HTTPS_PROXY is set and NODE_USE_ENV_PROXY isn't. Factually correct (the user's Node fetch is indeed ignoring their proxy), but if the true failure is unrelated to the proxy the hint may misdirect. Consider gating it to EAI_*/ECONNREFUSED/ETIMEDOUT codes where proxy misconfig is the plausible root cause.

CI

Head SHA f2d6ce3. All required checks green (Test, Typecheck, Lint, Build, CLI smoke (required), Fallow audit, SDK: unit + contract + smoke, Preview parity, preview-regression, regression, Render on windows-latest, Semantic PR title, Format, File size check, CLI: npx shim for ubuntu/macos/windows, CodeQL analyses). Tests on windows-latest still in progress at review time; its ubuntu counterpart passed and the same tests ran green on Render on windows-latest, so no risk of divergence.

— Via

[james-russo-rames-d-jusso]

Reviewed at f2d6ce32.

Small, well-scoped fix. Nothing blocking from my side — leaving as a comment.

What lands cleanly. The retry policy is a positive-list at the call-site, not an error classifier — PUBLISH_TRANSPORT_ATTEMPTS is passed only to uploadArchiveToPresignedUrl (:568) and to the staged-prepare call (:595), while publishProjectArchiveDirect (:543) and the staged /publish/complete (:627) both default to attempts = 1. That's the right shape here: the "which stages retry" decision lives in code you can grep, not in a shouldRetry(err) table that has to be kept in sync with Node's evolving fetch error taxonomy. The non-retry of /publish/complete has the exact "backend consumes and deletes the staged object, so blindly retrying it could duplicate an anonymous project or retry a publish that already committed" rationale in the body — the correct call, and the test at publishProject.test.ts:838 pins it (one rejection → immediate throw, no second call). S3 PUT is idempotent under key-replace so 2× is safe; prepare mints a fresh presigned URL so multiple prepares just leave one used and the others unclaimed (server-side ephemeral-state accumulation is not this PR's problem).

Load-bearing observability details:

• describeFetchFailure (:194-203) extracts the code from BOTH error.cause and the outer error, and skips the cause message when it duplicates the outer (causeMessage && causeMessage !== message). So "fetch failed" with cause "fetch failed" doesn't produce "fetch failed (fetch failed)" — nice touch.
• redactUrlQuery regex /(https?:\/\/[^\s?]+)\?[^\s]+/gu (:175-177) is light-touch — keeps the host+path visible (which is the diagnostic value: "the S3 hostname is unreachable") while stripping the Sig-V4 params. Test at :781 explicitly asserts not.toThrow("do-not-print") against a canary secret, which is the kind of test that catches a future "we accidentally leaked X-Amz-Signature to the terminal on a customer's zoom screenshare" regression.
• proxySupportHint (:179-192) checks both process.execArgv (child spawned with --use-env-proxy) and NODE_OPTIONS (env-configured). The NODE_OPTIONS parse is split(/\s+/u).includes("--use-env-proxy") — exact token match, so a hypothetical NODE_OPTIONS="--use-env-proxy-something-else" wouldn't false-positive. Good.
• createInit factory pattern (:207) so each fetch attempt gets a fresh AbortSignal.timeout and a fresh body. Without that, the second attempt would inherit an already-fired timeout signal and (for the S3 PUT) an already-consumed Blob. Easy trap to miss.

A few observations, non-blockers:

• Zero backoff between attempts (:211-222). For DNS/TCP transient failures, an immediate retry can hit the same negative-cache TTL or the same brief network glitch — a small delay (100-250ms) tends to convert marginal successes without noticeably lengthening the failure path (2 attempts × 100ms = 200ms added latency on the "definitely-broken" path). Worth considering as a follow-up; the current 2-attempt no-backoff shape is defensible and simpler and doesn't need useFakeTimers in tests.
• Timeouts are also retried. AbortSignal.timeout firings surface as AbortError, which the negative-list catch retries — so on prepare we get 2 × 30s = 60s worst-case wait, and on the S3 PUT 2 × uploadTimeoutMs. For a 500MB archive at the 500kbps floor that's 2 × ~1000s ≈ 33min before the user sees the diagnostic. Probably intentional (the alternative is to distinguish AbortError from TypeError in the catch and skip retry on timeouts), but worth naming — if #2840-style rollout instrumentation later shows a cohort waiting 30min on legitimately-dead servers, that's the loop to break. Non-blocker; today's shape is fine.
• Unreachable throw at :223. The loop always either returns or throws inside the if (attempt === attempts) branch, so the final throw new Error(failureStage) can never fire. TypeScript's control-flow analysis doesn't know that; keeping it as-is is fine. A tiny signal to a future reader would be throw new Error("unreachable: fetchForPublish loop exited without returning") or an assertion — bikeshed, ignore if you want.
• Live production verification carries weight. PR body says hyperframes@0.7.98 was exercised end-to-end AND a forced unreachable API reproduced the old fetch failed path plus the new stage-specific output. That's the pair of tests the unit suite can't do on its own for a network-boundary CLI. The screenshot on the Slack thread (Dinesh Kumar's Publish failed / fetch failed output) is the exact "totally blind" case this closes.

Test coverage at publishProject.test.ts:739-856 reads cleanly — retry-once-succeed on upload (:739), retry-cap-exhausted-throws with redaction pinned (:763), proxy-hint in error (:788), retry-once-succeed on prepare (:810), complete-does-not-retry (:838). Five tests for a ~75-line fix, each nailing a specific contract. Nothing missing that I can see.

LGTM from my side — left as a comment.

— Review by Rames D Jusso

[miguel-heygen]

Folded the full review sweep into dd629697d:

• added a deterministic 200ms delay before the one transport retry, so DNS/socket state gets a chance to clear;
• kept TimeoutError/AbortError single-shot, avoiding a doubled 60s metadata timeout or a doubled large-archive upload ceiling;
• replaced the unreachable loop fallthrough with one post-loop error path that reports the actual attempts made;
• added syscall and errno to the preserved Node system-error metadata;
• reworded the proxy hint to state that Node is ignoring configured proxy variables and make the remediation conditional on the network requiring them.

TDD evidence: four expected failures before the implementation; then focused 37/37, full CLI 2,481 passed / 2 skipped, typecheck, format, lint, and diff check all green. Please refresh the approval on exact head dd629697d after CI settles.

[james-russo-rames-d-jusso]

Reviewed at dd629697d.

All three of the R1 observations folded cleanly. Delta-verifying each:

Backoff. PUBLISH_RETRY_DELAY_MS = 200 + waitBeforePublishRetry() (:230-232) called between attempts. Test at :767 uses vi.useFakeTimers() with a three-step timeline — assert 2 calls at t=0 (prep + first-attempt), still 2 at t=199, jump to 4 at t=200 (retry proceeded through S3 PUT + finalize). That's the tightest way to pin a "waited exactly N ms" contract; the alternative (spy on setTimeout) would test the mechanism instead of the observable behavior. 200ms is a defensible starting point — long enough that DNS negative caches likely dropped, short enough to be imperceptible on the "definitely-broken" path.

Timeouts short-circuit. isRequestTimeout (:214-218) catches both DOMException names that AbortSignal.timeout can surface as (TimeoutError and AbortError), and the retry loop breaks out immediately without sleeping (:239). Test at :883 pins the shape — one fetch call, error message Failed to prepare project upload: The operation was aborted due to timeout (no "after N attempts" suffix because attemptsMade === 1). Right call — a timeout means we already spent timeoutMs waiting; retrying would just re-consume that budget on the same failure.

Throw is now reachable. The old inside-catch throw is replaced by outside-the-loop throw at :245-247, driven by lastError and attemptsMade tracked through the loop. TypeScript can now see the throw is the loop's only exit-with-error, and the RangeError bounds check at :225 (attempts < 1) closes the defensive gap. Nice.

Bonuses I didn't ask for:

• systemErrorMetadata (:171-180) now captures syscall and errno alongside code, deduplicated across cause + outer. Test at :809 pins the resulting shape — (EAI_AGAIN, syscall=getaddrinfo, errno=-3001: getaddrinfo EAI_AGAIN s3.example.com) — which is the diagnostic uplift that turns "some DNS thing failed" into "getaddrinfo call, EAI_AGAIN, errno -3001" that a support engineer can grep against a distro's netdb.h if it comes to that.
• Proxy-hint phrasing tightened (:196) — "set but ignored by Node fetch; if this network requires them" replaces "disabled". More accurate: the runtime isn't broken, it just doesn't opt-in to proxies without the flag.

Test coverage delta at publishProject.test.ts is proportional — two new tests (:767 for the backoff timing, :883 for the no-retry-on-timeout) plus the exhausted-retry test's expected message updated to reflect the new metadata format. networkFailure helper extended with an optional metadata param, and only the tests that exercise metadata pass it — no shotgun changes.

LGTM from my side — clean fold.

— Review by Rames D Jusso

[vanceingalls]

R2 delta-verify — fix(cli): recover transient publish failures

Verdict: APPROVE

Delta reviewed: f2d6ce3 → dd629697d. Only packages/cli/src/utils/publishProject.ts and its test file changed. Standing R1 APPROVE upgraded to R2 APPROVE.

R1 nits — status

1. Backoff between attempts — addressed. New PUBLISH_RETRY_DELAY_MS = 200 + waitBeforePublishRetry() inserted between attempts inside fetchForPublish. Total wait bounded (single retry → 200ms max additional latency). No jitter, but this is a per-user CLI publish path — there's no fleet of concurrent retriers to synchronize, so a fixed constant is fine. New test waits briefly before retrying a transport failure pins the 200ms boundary with fake-timer assertions at 199ms (2 calls) → 1ms later (4 calls). Semantic, not presence.

2. Unreachable throw after the loop — addressed by structural refactor. The loop now breaks on last attempt (or on timeout, see below), sets lastError / attemptsMade, and the throw lives after the loop as the terminal path. Dead code gone; the RangeError guard on attempts < 1 is a nice-to-have addition.

3. errno / syscall on system errors — addressed. errorCode() replaced with systemErrorMetadata() which reads code, syscall, errno. No PII surface: these are Node system-error primitives (getaddrinfo, -3001, EAI_AGAIN) — no paths, no user input, no tokens. Dedup filter across cause + top-level. Output still runs through redactUrlQuery. Existing PII-leak assertion (.not.toThrow("do-not-print")) still holds — verified in the updated test that asserts the exact new format (EAI_AGAIN, syscall=getaddrinfo, errno=-3001: getaddrinfo EAI_AGAIN s3.example.com).

4. Proxy hint gating — addressed via wording, not error-class gating. proxySupportHint() still fires whenever HTTPS_PROXY is set, but the copy is now hedged: "Proxy variables are set but ignored by Node fetch; if this network requires them, retry with NODE_USE_ENV_PROXY=1". Not the class-gate I sketched, but functionally equivalent — the user is no longer told definitively that the failure is proxy-related. Acceptable resolution for a nit.

Adversarial delta pass

• New retry classification (isRequestTimeout): DOMException with name === "TimeoutError" || "AbortError" now short-circuits the retry loop. Behavior change: previously a timeout on attempt 1 would burn a retry; now it fails fast. Correct — retrying a timeout with the same timeout budget is pathological. New test does not retry a request that reached its timeout pins this to 1 fetch call. AbortError also short-circuits — safe because this code owns the AbortSignal (from AbortSignal.timeout), so any AbortError is a self-imposed timeout, not an external abort.

• Retry-transient-only invariant: retry surface is still bounded by PUBLISH_TRANSPORT_ATTEMPTS = 2 and now further narrowed by isRequestTimeout. Nothing widens the retry class.

• Idempotency: retry scope unchanged (was 2 attempts before, still is). The prepare/upload/finalize endpoints' idempotency assumptions are the same as at R1.

• Test drift check: no assert-presence regression. All new / modified tests pin exact message strings (with the new syscall=/errno= metadata), exact fetch-call counts, exact timer-advance boundaries.

• Cross-platform: DOMException, AbortSignal.timeout, setTimeout all universal in Node 22. syscall/errno are Node system-error fields on all platforms — Windows will emit different codes (ETIMEDOUT, ENETUNREACH variants) but the metadata plumbing is agnostic.

CI

Green on all required checks except Tests on windows-latest (pending — non-required parity to the passing Test job). Non-blocking.

— Via