Repository navigation
Conversation
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Approved, with one non-blocking maintenance suggestion: share the runtime-neutral deadline helper and its regression tests between Spin and Cloudflare to keep future race fixes aligned, while retaining runtime-specific timers and cancellation in each adapter.
dhruv8sh
left a comment
There was a problem hiding this comment.
This is a careful, well-tested rework of auction deadline handling. Replacing the dormant has_enforceable_total_request_deadline capability with a single select_until / wait_until contract that every adapter implements is the right shape. The default implementation refuses with Unsupported instead of falling back to an unbounded wait, which is the correct safety default. The nonblocking snapshot on expiry keeps bids that already arrived when the publisher origin is slow, and the mediator cutoff (the earlier of the auction deadline and the mediator's own timeout) is now checked at all three points in both the sync and split paths.
The adapter work holds up on a close read:
- Fastly: the timed
fastly_async_io::selectgets a positive timeout (.max(1)), so the host can never treat it as unbounded. Each partial body is capped, and so is the total across bodies. Invalid handles fail only their own provider. - Axum: handles are borrowed across
tokio::time::timeout, so expiry can't drop siblings that already finished. - Cloudflare and Spin: the final
try_recvcloses the race between the timer and the response channel.
The real-runtime tests under Viceroy, workerd and Spin are a big step up from stub-only coverage.
CI status: all checks pass.
None of the comments below is blocking. The one I'd most like a reply on is the Fastly-only cache-bypass rule (comment 1).
Share pending-operation ownership and deadline channel collection between Cloudflare and Spin while retaining runtime timers and cancellation. Require cache bypass in bounded HTTP test collection like Fastly, preserve healthy siblings, and update auction test providers. Replace four elapsed wall-clock assertions with checks that unbounded work was skipped. Clarify bid eligibility and coordinated Fastly SDK updates.
Preserve current creative asset/click settings, renderer diagnostics, and CI Node/domain checks alongside the reviewed deadline changes. Resolve the adapter lockfile entries by retaining web-time and main's removal of redundant direct JS dependencies. Keep the new click-rewrite configuration row and the original dispatch-deadline description.
Summary
Changes
core/src/auction/orchestrator.rsCrate paths above are relative to
crates/trusted-server-.fastly-sysis inherited from workspace dependencies at the locked SDK version;web-timesupplies the shared clock.Closes
Closes #1015
Spec
Not a feature. Implements #1015. Technical plan:
docs/superpowers/plans/2026-10-22-absolute-auction-deadlines.md.Deadline decisions and limits
Provider bids complete at the final nonblocking snapshot remain eligible even after
tmax; mediation starts only with remaining budget and rejects responses collected after its cutoff. This bounds additional network waiting rather than eligibility by an unavailable upstream arrival timestamp.Fastly does not use an empty handle list or millisecond header polling. One missing cache-bypass marker or malformed pending request fails only that provider, retaining healthy siblings. Retained payloads share a 10 MiB cap; pressure rejects the largest partial response under its own name. Payload vectors move directly into platform bodies. This cap excludes host/allocator overhead; the final nonblocking drain uses local CPU bounded by payload size, not a separate CPU-time ceiling.
Fastly closes acquired bodies and abandons pending requests because its installed SDK cannot cancel them. Other adapters abort owned tasks; Cloudflare also aborts fetch. Upstream side effects cannot be undone. Runtime scheduling and timer granularity can add overshoot; local parsing, ranking, HTML work, and delivery remain outside the network cutoff.
Review findings and proof
Test plan
Exploratory
tsc --noEmitand whole-repositoryactionlintreport pre-existing errors in unchanged files; they are not counted as passing checks. Local verification uses Rust 1.98.1, Viceroy 0.21.0, and Node 24.12.0; the repository pins Node 24.20.0. Runtime checks use local SDK runtimes, not production Fastly/Cloudflare deployments.Checklist