Register APS renderer via requestId so it survives Prebid field stripping

[aram356]

Fixes #962. Stacked on #918 (targets issue-764-aps-openrtb).

Problem

APS bid-by-reference renderers never register, so every APS bid renders empty and Prebid throws Missing ad markup or URL (reason noAd).

interpretResponse sets the renderer descriptor on the bid as the custom trustedServerRenderer field; the bidResponse listener registers it into window.tsjs.apsPrebidRenderers keyed by the generated adId. But Prebid drops unknown top-level fields when it normalizes bids during addBidResponse, so the custom field is gone before the listener runs (absent as early as bidAccepted). The listener sees renderer === undefined, returns, and the registry stays empty.

Fix (minimal, additive)

Stash the descriptor keyed by requestId — a first-class field Prebid preserves — when the bids are built, and have the bidResponse listener fall back to that stash when the custom field is absent.

• interpretResponse (auctionBidsToPrebidBids): if (renderer) stashPendingApsRenderer(requestId, renderer)
• bidResponse listener: const renderer = bid[APS_RENDERER_FIELD] ?? takePendingApsRenderer(bid['requestId'])
• Bounded Map (256 entries); take deletes on read.

Additive by design: the existing custom-field path is unchanged, so cases where the field survives (e.g. unit/browser harness) behave exactly as before. The fallback engages only when Prebid has stripped the field. No new dependency; the bridge, descriptor format, server, and render path are untouched.

Verification

• New unit test: descriptor stashed, custom field removed (simulating Prebid), registration still succeeds via requestId.
• Full JS suite passes; formatting clean.
• Validated against a live deployment by serving the built bundle: renderer registrations went from 0 to non-zero, APS noAd failures dropped sharply, and APS creatives rendered in-slot instead of erroring.

[ChristianPavilonis]

Summary

The requestId fallback is directionally correct, but this PR is not safe to merge in its current state. The stacked base was rebased: GitHub reports the PR as conflicting, the merge base is stale, and the displayed change set has expanded to 43 files rather than the intended two-file fix. The head omits current-base commits covering publisher streaming, inline SSAT rendering, APS security review fixes, and rebased CI fixes; the final-tree comparison removes approximately 1,426 lines from streaming_processor.rs alone.

Please recreate or rebase this branch from the current origin/issue-764-aps-openrtb, then cherry-pick or reapply only 7406ed7d. Confirm that the resulting PR contains only the intended Prebid implementation and test changes, and rerun the complete CI gates. The currently green integration checks predate the latest base commits and do not validate the conflicting merge result.

[ChristianPavilonis]

🔧 The stale head reintroduces client-visible leakage of all APS upstream response headers

debug_headers copies every upstream response header into APS debug metadata, which is returned to page JavaScript. This can expose Set-Cookie, identity headers, internal tracing data, or authentication-related metadata whenever APS debugging is enabled. The current base intentionally fixes this with a fail-closed allowlist containing only Content-Type.

Please preserve that allowlist while rebasing. Any additional debug headers should be explicitly reviewed and redacted rather than copied wholesale.

[ChristianPavilonis]

🔧 Pending renderer descriptors are not consumed when the custom field survives

Because ?? short-circuits, takePendingApsRenderer is never called when trustedServerRenderer survives normalization. The descriptor stashed earlier therefore remains until FIFO eviction. A renderer envelope may contain up to 349,528 base64 characters, so retaining 256 entries can preserve roughly 85 Mi characters on a long-lived publisher or SPA page; it also leaves stale descriptors available if a request ID is reused.

Consume the pending value before selecting the primary/fallback renderer:

Suggested change

	const renderer = bid[APS_RENDERER_FIELD] ?? takePendingApsRenderer(bid['requestId']);
	const pendingRenderer = takePendingApsRenderer(bid['requestId']);
	const renderer = bid[APS_RENDERER_FIELD] ?? pendingRenderer;

Please also add a regression test that registers through the surviving custom-field path, then sends a field-stripped event with the same request ID and verifies that no stale renderer is registered again.

[prk-Jr]

Summary

The root cause is well diagnosed and the fix is the right shape: requestId is a first-class field Prebid preserves, the fallback is additive, and the new test simulates the actual stripping rather than asserting on internals. Three things block merge: the PR cannot currently be merged into its base, the descriptor scrub was quietly narrowed to the success path, and the stash key is per-adUnit rather than per-bid so multiple APS bids on one imp lose all but the first renderer.

Note on review surface: the substantive change is one commit and two files (crates/trusted-server-js/lib/src/integrations/prebid/index.ts and its test). The other 41 files in the GitHub diff are an artifact of the base branch having been rebased under this branch — see the first finding.

Blocking

🔧 wrench

• PR is unmergeable into its base: GitHub reports mergeable: CONFLICTING / mergeStateStatus: DIRTY. issue-764-aps-openrtb was rebased after this branch merged an older copy of it, so a test merge conflicts in 8 files: auction/orchestrator.rs, integrations/aps.rs, integrations/prebid.rs, publisher.rs, browser/tests/shared/aps-renderer.spec.ts, gpt/index.ts, prebid/index.ts, prebid/index.test.ts. Both files this PR actually changes are in that list, and the base's bidResponse listener still has the pre-fix shape, so the conflict resolution is not mechanical. Merge the current base in and re-verify the fix still applies on top of it.
• Descriptor scrub became conditional: delete bid[APS_RENDERER_FIELD] now only runs when registration succeeds, so a failed registration leaves the descriptor on the Prebid bid object (prebid/index.ts:579). Inline comment has the detail.
• Stash key collides on multiple APS bids per imp: requestId is per-adUnit, and takePendingApsRenderer deletes on read, so only the first bidResponse for a given imp registers (prebid/index.ts:254). Inline comment has the detail.

Non-blocking

♻️ refactor

• Consider bid.meta over a module-global Map: meta is first-class, already populated by this adapter, and preserved through addBidResponse — it would remove the stash, the cap, the eviction, and the key collision above (prebid/index.ts:204).

🤔 thinking

• Stash lifetime: entries are left behind both when the custom field survives (?? short-circuits before take) and when Prebid rejects a bid before delivery. Bounded by the 256 cap, but clearing on auctionEnd would scope the map to the auction it belongs to (prebid/index.ts:564).

👍 praise

• Root-cause writeup and test design: the commit message pins the failure precisely (field absent as early as bidAccepted, not merely at bidResponse), and the test removes the custom field to reproduce stripping instead of asserting on the stash internals. registerApsPrebidRenderer re-validating the descriptor (aps/render.ts:210) also means the new path inherits full validation rather than trusting the stash — good layering.

CI Status

Four checks ran and all pass: browser integration tests, integration tests, integration tests (Fastly EC lifecycle), prepare integration artifacts.

The repository's main gates — cargo fmt, the six clippy-* targets, the four cargo test-* targets, the parity suite, and the JS build/test/format jobs — did not run on this PR, since it targets issue-764-aps-openrtb rather than main. Given the change is JS-only, please confirm npx vitest run and npm run format in crates/trusted-server-js/lib were run locally against the post-merge tree.

[prk-Jr]

🔧 wrench — Stash key collides when one imp gets multiple APS bids.

requestId here is origReq?.bidId ?? bid.impid — a per-adUnit value, not per-bid. parseAuctionResponse flattens every seatbid[].bid[] (src/core/auction.ts), so two or more APS bids for the same impid map to the same requestId. The second stashPendingApsRenderer call overwrites the first, and takePendingApsRenderer deletes on read — so the first bidResponse consumes the single entry and the second bid registers nothing.

If the auction winner is that second bid, the slot renders empty and Prebid throws Missing ad markup or URL — the exact failure this PR fixes. The old custom-field path did not have this collision because each normalized bid object carried its own copy.

Fix: key the stash per bid rather than per request, or store a list per requestId:

// per-bid key
const rendererKey = `${requestId}|${bid.creativeId}|${index}`;
if (renderer) stashPendingApsRenderer(rendererKey, renderer);

…which requires the key to be reconstructible in the bidResponse listener, so a list-per-requestId (shift on read) is likely the smaller change.

[prk-Jr]

🔧 wrench — The descriptor scrub silently became conditional.

Before this PR the listener deleted bid[APS_RENDERER_FIELD] unconditionally, immediately after the registration attempt. Now the delete only runs on the success branch, so a descriptor that fails registration stays attached to the Prebid bid object and travels onward into targeting and analytics consumers.

That contradicts the comment directly below it ("Keep the executable capability only in the bounded, one-time registry") and the PR description's claim that the existing custom-field path is unchanged.

Fix: restore the unconditional scrub.

const registered = registerApsPrebidRenderer(/* … */);
delete bid[APS_RENDERER_FIELD];
if (!registered) {
  log.warn('[tsjs-prebid] rejected APS renderer capability that failed registration');
}

[prk-Jr]

♻️ refactor — bid.meta may remove the need for module-global state entirely.

meta is a first-class Prebid bid field that this adapter already populates a few lines down (meta: { advertiserDomains: bid.adomain }) and that Prebid preserves through addBidResponse — the same property requestId is being relied on for here. Carrying the descriptor as meta.trustedServerRenderer would eliminate this Map, the 256-entry cap, the FIFO eviction, the take-on-read lifecycle, and the key-collision problem flagged on the stash call below.

Worth confirming against the same live deployment used to validate this fix before committing to module-global state, since the stripping behaviour was observed empirically rather than derived from Prebid's contract.

[prk-Jr]

🤔 thinking — Two stash-lifetime leaks, both bounded but avoidable.

1. When the custom field does survive, ?? short-circuits and the stashed copy for that requestId is never taken — it lingers until FIFO eviction. Taking unconditionally (or deleting after a successful registration) would keep the map tight.
2. Bids Prebid rejects before delivery (below floor, size filtered, invalid) never reach this listener at all, so their descriptors are also held until eviction.

Neither is unbounded, but clearing the map on auctionEnd would collapse both cases and make the map's lifetime match the auction it belongs to.