fix(deploy): retry peer-side payload_blob read on transient 503 stall

[kriszyp]

Summary

The peer branch of deploy_component's replicated payload read (components/operations.js) gave the blob content stream a single 20s no-progress window (Blob.stream()'s blobReadTimeout) before failing the whole deploy. That 503 BlobReadError is documented as retryable (resources/blob.ts) and is usually self-healing, but nothing on the deploy path retried it.

readPayloadBlobWithRetry (components/deploymentRecorder.ts) retries a fresh stream() on the retryable 503 with backoff, bounded by the same timeoutMs budget already used to wait for the hdb_deployment row (one shared deadline covers both phases, so a slow row doesn't double the peer's worst-case wait). It only retries while nothing has reached the consumer yet — a stall after partial content fails immediately, avoiding duplicate bytes on a naive restart — and streams chunks through via an async generator + Readable.from() rather than buffering, preserving backpressure for large payloads. 500 corrupt / 404 gone errors still fail immediately, unchanged from before.

Purpose

Root-caused from a harper-pro incident (2026-07-16): an origin replicated the hdb_deployment row (peer wrote the blob header), but content bytes didn't arrive for 60+ seconds despite a live replication connection and no shutdown drain — the peer's one-shot 20s window failed the deploy, and the content landed seconds later (a manual redeploy 17 minutes later succeeded). This converts that failure class to success regardless of the upstream cause (suspected per-connection blob-send backpressure or event-loop starvation on a single-worker origin).

This stays on main/5.2 for now — no v5.1 cherry-pick yet (per Kris): the next 5.1.x patch carries only the diagnostic visibility change (HarperFast/harper-pro#591) to root-cause the send starvation with minimal added complexity; this fix's 5.1 backport gets revisited after that diagnosis (removal follow-up: HarperFast/harper-pro#592).

Where to look

• components/deploymentRecorder.ts: readPayloadBlobWithRetry / readPayloadBlobChunks — the retry+backpressure+cancellation design. Worth reading the JSDoc; it documents two non-obvious things a first-pass implementation got wrong (caught during review, see below) before landing on the current shape.
• components/operations.js (~line 465): the peer branch call site — shared deadline math.
• resources/blob.ts: one-line change, exporting the existing BLOB_UNAVAILABLE_STATUS constant so the retry helper doesn't duplicate the magic number.

Review notes for the human reviewer

This went through a cross-model review (Codex + Gemini + a Harper-domain pass) across two iterations. Two real issues were caught and fixed in the version you're looking at now, noted here since they reflect real design tradeoffs rather than things left open:

1. An earlier revision buffered the entire payload into memory per retry attempt (Buffer.concat) — flagged as a regression for this repo's supported multi-GB deploy path. Fixed by switching to a pull-based async function* + Readable.from(), which also turned out to matter for a second, subtler reason: a naive ReadableStream wrapper that does for await (...) controller.enqueue(chunk) ignores desiredSize and eagerly drains the source regardless of consumer demand, silently reintroducing the same unbounded-memory problem even without an explicit buffer. Covered by a unit test that asserts the source isn't drained ahead of consumer demand.
2. Readable.from()'s cancellation (return() on destroy()) only takes effect at the generator's next yield — which never happens while the loop is stuck retrying. Verified empirically that destroy() was a no-op until the retry loop's own deadline expired. Fixed by threading the constructed Readable back into the generator (via a mutable cell, since it doesn't exist yet at generator-creation time) so it can check .destroyed explicitly each iteration.

One thing left deliberately out of scope: neither this fix nor the pre-existing code cross-validates the read bytes against row.payload_hash/payload_size (the row already carries a SHA-256 from ingestPayload). Confirmed via git show HEAD~1 that this integrity gap predates this diff on both the origin and peer paths — raising it here as a known gap, not something this PR should absorb given the intentionally narrow, cherry-pickable scope.

Tests

• unitTests/components/deploymentRecorder.test.js — 10 new cases for readPayloadBlobWithRetry: 503-then-success, 503-forever-fails-after-budget, 500-fails-immediately, no-statusCode-fails-immediately, no-retry-after-partial-forward, zero-timeout, backpressure (pull-based, doesn't drain ahead of demand), and bounded retries after destroy().
• Ran the full deploy-* integration suite (tracking, peer-branch, multipart-stream, payload-reclaim) plus test:unit:main / test:unit:resources — all green aside from pre-existing, unrelated flakes (globalIsolation.test.js sandbox-path issue specific to this worktree's node_modules layout, and one order-dependent tokenAuthentication.test.js failure that passes cleanly in isolation).

---

Generated by Claude Opus (dev-agent), via Kris's dispatch queue.

🤖 Generated with Claude Code

[claude]

Reviewed; no blockers found. Remaining suggestion (credentials timeout coercion duplication) has been addressed.

[gemini-code-assist]

Code Review

This pull request introduces a robust retry mechanism for reading replicated deployment payloads by implementing readPayloadBlobWithRetry. This helper function handles transient 503 errors (blob stalls) by retrying the stream read with exponential backoff, while ensuring that backpressure is correctly maintained and that retries are safely bounded to prevent data duplication or unnecessary resource consumption. Comprehensive unit tests have been added to verify the retry logic, cancellation behavior, and streaming performance. I have no feedback to provide as there were no review comments.