Name the guard that stops an APS creative render

[jevansnyc]

Why

Investigating blank ads on a live publisher page, APS bids were winning the
auction and Ad Manager was filling the slot, yet the creative never drew. The
tester framework reported the slot as "filled" at 1x1, which is exactly what a
successful universal-creative render looks like before it resizes. Nothing
downstream distinguished the two.

Two blind spots made this close to undiagnosable from the outside:

• Every guard on the APS render path returned silently. A frame that rejected
  the descriptor, one that never received it, and one that timed out all looked
  identical: an iframe that loaded and did nothing.
• The APS capability handshake never reported to GPT diagnostics, so ts_console
  showed delivery: unknown on every request cycle and zero creative failures,
  while APS bids were rendering blank.

On the page under investigation that was 24 of 24 cycles unattributed.

What changed

The sandboxed renderer document (aps.rs) reports which guard stopped it,
on the existing failure message: bad_hash, source_mismatch,
nonce_mismatch, descriptor_keys, descriptor_fields,
descriptor_envelope, amazon_script_error.

The Universal Creative source (render.ts) labels its own frame_timeout
and frame_load_error and relays whichever reason it holds to the top window.

The GPT bridge (gpt/index.ts) records a creative attempt around the
handshake and names each silent return: aps_consumed_tombstone,
aps_source_not_in_ad_unit, aps_descriptor_fields, aps_tombstone_capacity,
aps_missing_renderer_url. A successful post records a response, so this path
reports trusted_server_response_sent rather than unknown.

Notes for review

The APS path runs on the publisher's own Prebid ad units, which never pass
through Trusted Server slot mapping. No creative opportunity exists for them, so
the store rejected the attempt as creative_request_without_slot. The bridge now
resolves the GPT slot by element ID and records the opportunity first. That is
the least obvious part of the change and the part most worth a look.

Security posture, since a reason crosses an origin boundary:

• Reasons are fixed categories. A rejected descriptor is never echoed back.
• Reporting from the frame is one-shot and answers through parent, never the
  sender, so an unrelated sender cannot consume the report or learn from it.
  Traffic not shaped like the handshake stays silent, as before.
• Relayed reasons resolve through a null-prototype allowlist, so a hostile
  __proto__, constructor, or toString resolves to undefined.
• The attempt is taken from our own tombstone, never from the message.
• The relay is recording only and never influences delivery.

This is instrumentation. It does not attempt a fix, because the root cause is
still unknown: the evidence says the handshake completes and the renderer frame
loads, then the chain dies before Amazon's prebid-creative.js is requested.
These reason codes are what will name it on the next occurrence.

Testing

• render.test.ts: relay of a frame reason, frame timeout, and the allowlist
  including inherited-key and non-string rejection
• ad_init.test.ts: creative attempt recorded for a registered APS renderer,
  and the tombstone reason on a replayed ad ID
• aps.rs: every guard reports a reason, reporting is one-shot, no descriptor
  echo, and the frame never answers the sender

Gates run: cargo fmt --check, clippy (fastly target, -D warnings),
cargo test -p trusted-server-core (2143 passed), JS build, JS format, and the
full vitest suite (850 passed).

Two caveats. The vitest suite has 27 pre-existing failures in
sourcepoint/index.test.ts and permutive/segments.test.ts, all one
localStorage error from running Node 26 against a repo pinned to 24.12.0; the
count is unchanged by this branch. And the remaining adapter gates (axum,
cloudflare, spin, parity) were not run locally, because a .cargo/config.toml
alias conflict with a stale sibling checkout meant cargo had to be invoked from
outside the repo without workspace aliases. CI covers those.

[aram356]

@aram356 Will test in staging before merging in #1019

[aram356]

@ChristianPavilonis to understand if belongs in #1019

[ChristianPavilonis]

Summary

Reviewed cb1de4777cdd5efe2a32892c03d00c1b071795f6. Requesting changes because the new reporting path can alter APS delivery and the diagnostics pipeline does not currently retain the APS failure evidence this PR introduces. Four actionable findings are posted inline.

[prk-Jr]

Summary

Instrumentation for the APS Universal Creative render path: each silent guard now names itself, the creative frame relays its reason to the top window through a null-prototype allowlist, and the GPT bridge opens a diagnostics attempt around the capability handshake. The security reasoning in the description is unusually careful and the allowlist is the right shape for a cross-origin relay.

Three defects block it, all verified with runnable probes rather than read off the diff. The headline one is that none of the fifteen new reason codes can reach the store: isCreativeFailure in store.ts still allowlists only the original four, so every recordTrustedServerCreativeFailure(attemptId, 'aps_*') call added here is a no-op. ts_console will still report zero creative failures on the APS path, which is the exact blind spot the PR exists to close. Separately, adding a reason key to the renderer document's failure message breaks the direct (non-Prebid) render path, which gates on an exact two-key match.

1 of the inline comments below carries a one-click GitHub suggestion — use Commit suggestion to apply it as a commit on the PR branch. The remaining comments describe the fix in prose because the change touches files or lines outside this diff and can't be auto-applied.

Blocking

🔧 wrench

• Every new aps_* reason is dropped before it reaches the store — see inline at crates/trusted-server-js/lib/src/core/types.ts:213
• creativeFailureFact is non-exhaustive; ts_console will render undefined — see inline at crates/trusted-server-js/lib/src/core/types.ts:229
• Direct APS render path stops tearing down on renderer-failed — see inline at crates/trusted-server-core/src/integrations/aps.rs:60
• The new bridge tests mock the recorder, so they cannot catch the store gap — see inline at crates/trusted-server-js/lib/test/integrations/gpt/ad_init.test.ts:3379
• The relay branch has no test — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1725

Non-blocking

🤔 thinking / ♻️ refactor / 📝 note

• source_mismatch burns the one-shot report and is attacker-triggerable — see inline at crates/trusted-server-core/src/integrations/aps.rs:103
• The opportunity lands on the slot's next request cycle, mislabeling it — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1619
• beginApsCreativeAttempt runs before the source check — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1760
• window.googletag untyped access — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1616 (carries a suggestion)

👍 praise

• Null-prototype allowlist, and tests that actually prove it — see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:56

Cross-cutting / body-level findings

• 📝 New type errors introduced, though tsc is not a gate here. A tsc --noEmit delta between the merge-base and this head shows 8 new errors: the overlay.ts TS2366 and the gpt/index.ts TS2339 covered inline, plus 6 × TS2532 (Object is possibly 'undefined') in ad_init.test.ts. The base already carries 289 errors so this is not a regression in gate terms, but the first two are pointing at real defects — worth noting that the type system did flag both of the JS-side bugs found in this review, and nothing was listening.

CI Status

• browser integration tests: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo fmt: PASS
• cargo test: PASS
• cargo test (axum native): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• format-docs: PASS
• format-typescript: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• prepare integration artifacts: PASS
• vitest: PASS

All 14 checks green. Every finding below survives green CI — noted in each comment where the existing tests structurally cannot catch the defect.

[aram356]

Summary

This is careful, well-documented instrumentation, and the reasoning in the PR description about the security posture of the relay is sound. But the change does not currently deliver the outcome it is written for: I verified against a real GptDiagnosticsStore that every one of the 15 new aps_* reasons is discarded before it reaches a diagnostics record, so a blank APS render still reports delivery: unknown. Two further defects sit behind that one, and one of them is a behavioural regression on the direct render path rather than a diagnostics-only concern.

The full JS suite passes on this branch (899/899). That is precisely the problem: the new tests mock gptDiagnosticsRecorder with vi.fn()s and never exercise the store, so the gap between the widened type union and the store's runtime guard is invisible to them.

A note on the PR description: the 27 vitest failures you reported are not reproducible here. On the repo-pinned Node 24.12.0 the suite is fully green, which matches your own diagnosis that they were a Node 26 artifact.

None of the inline comments below carry a one-click suggestion. Each fix either lands in a file outside this diff (store.ts), touches lines outside a diff hunk, or needs a design decision, so all of them describe the change in prose instead.

Blocking

wrench

• The store rejects all 15 new aps_* reasons; nothing is ever recorded - see inline at crates/trusted-server-js/lib/src/core/types.ts:197
• Relayed frame failures land on a completed attempt and are dropped - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1809
• beginApsCreativeAttempt corrupts attribution for the next request cycle - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1616
• The new 3-key failure message breaks the direct render path - see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:771
• Reordering the source check lets a co-resident script cancel a successful render - see inline at crates/trusted-server-core/src/integrations/aps.rs:104

Non-blocking

thinking / nitpick

• renderable_candidate is semantically wrong on this path - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1618
• The relay handler applies no source validation - see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1723
• m.nonce===undefined widens what can cancel a pending render - see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:775
• The guard-coverage test is close to a tautology - see inline at crates/trusted-server-core/src/integrations/aps.rs:2629
• The server-side APS path gets no instrumentation - see the cross-cutting section below

Cross-cutting / body-level findings

• thinking - The server-side APS render path is left uninstrumented. installTsRenderBridge has two APS paths. This PR instruments the Prebid-capability path thoroughly, but the server-side path at gpt/index.ts:1848-1875 (reached when matchedBid.renderer !== undefined) still has the same silent returns the PR is written to eliminate: if (!renderer) return; and if (!rendererUrl) return false;, with no safelyRecordCreativeFailure on either. Given the PR's framing, a reader will assume APS render failures are now attributable in general, and on this path they are not. If the omission is deliberate because the live investigation only involved the publisher-Prebid path, that is a reasonable scope decision, but it is worth a sentence in the PR description so the next person debugging a blank server-APS slot is not misled by the reason codes' apparent coverage.

• note - How these findings were verified. Findings 1, 2, and 3 were each confirmed by running throwaway tests against the real GptDiagnosticsStore rather than by reading alone, and finding 4 by driving renderApsCreative end to end. The scratch tests were discarded; the observed outputs are quoted in the relevant inline comments. Gates run locally on this head: cargo fmt --check PASS, cargo clippy-fastly PASS, cargo test -p trusted-server-core 2255 passed / 0 failed, npx vitest run 899 passed / 0 failed, npm run format PASS.

CI Status

• No GitHub checks reported on aps-renderer-failure-diagnostics (gh pr checks returns "no checks reported on the branch"); not run remotely. Locally on this head: cargo fmt --check PASS, cargo clippy-fastly PASS, cargo test -p trusted-server-core PASS (2255), npx vitest run PASS (899), npm run format PASS. The remaining adapter gates (axum, cloudflare, spin, parity) were not run.

[ChristianPavilonis]

Summary

Reviewed 285e00372615359f22971e5af5df73470c98790c against 066ea3c69f5cb2e763ba739914e4f0c8861bd3a7. Requesting changes because the PR currently conflicts with main, and the additional P2 finding posted inline can still drop the delayed APS failure evidence this change is intended to retain. Existing findings from prior reviews are not repeated here.

[ChristianPavilonis]

🔧 P2: Keep the attempt correlation through the renderer failure window

A capability can be consumed while still valid but with less than ten seconds remaining. The Universal Creative may then report frame_timeout or another delayed failure, but this prune removes the tombstone before the following lookup can recover its attemptId. The blank render consequently loses its reason and remains unattributed. If the same ad ID is registered again after expiry, a late report from the old render can instead collide with the newer attempt.

The tombstone stores prebidRendererEntry.expiresAt at lines 1772-1776, while the renderer timeout fires after 10 seconds at render.ts:779. The existing passing test at ad_init.test.ts:3734 also confirms that an expired tombstone permits the same ad ID to be consumed again.

Retain the tombstone and attempt ID for at least the renderer's maximum failure-reporting window after consumption, or correlate the relay with a unique per-render token rather than the reusable ad ID. Please add a fake-timer test that consumes a capability just before expiry, advances through the renderer timeout, and verifies that the failure remains attached to its original attempt.

[aram356]

Summary

Instrumentation-only change that names the guard stopping an APS creative render. The security shape of the cross-origin relay is right: fixed reason categories, no descriptor echo, one-shot reporting, and a null-prototype frozen allowlist that the tests genuinely exercise (__proto__, constructor, toString, non-strings).

The problem is that the reasons never arrive. The runtime allowlist in the store still lists only the four pre-existing categories, so all fifteen new aps_* reasons are discarded before they are recorded — verified by running the real GptDiagnosticsStore. On top of that, the success path marks the attempt completed the moment the descriptor is posted, so a render failure relayed ten seconds later is dropped even once the allowlist is fixed. A slot that renders blank therefore reports renderable_candidate plus delivery: trusted_server_response_sent with no failures — a confident wrong answer where the pre-PR state was an honest unknown.

Separately, the renderer document's new three-key failure message broke the direct render path's teardown, which is a behavioural regression rather than a diagnostics gap.

CI is green on all 20 checks, which is itself a finding: the new bridge tests assert against a vi.fn() recorder, so nothing downstream of the call site is covered.

1 of the inline comments below carries a one-click GitHub suggestion. The rest describe the fix in prose because the change lands in files this PR does not touch, or on lines outside the diff, and cannot be auto-applied.

Blocking

🔧 wrench

• The store's runtime allowlist drops all fifteen new aps_* reasons — see inline at crates/trusted-server-js/lib/src/core/types.ts:197
• Recording the response closes the attempt, so every relayed frame failure is dropped — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1809
• The three-key failure message breaks the direct render path's teardown — see inline at crates/trusted-server-js/lib/src/integrations/aps/render.ts:771
• creativeFailureFact is non-exhaustive, so ts_console renders undefined — see inline at crates/trusted-server-js/lib/src/core/types.ts:214
• The relay branch has no test, which is why the two blockers above ship green — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1730
• The relay branch performs no source check on an untrusted message — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1722

Non-blocking

🤔 thinking / ♻️ refactor / 📝 note

• Untyped window.googletag introduces a new TS2339 — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1613 (suggestion)
• Recording the opportunity at render time mis-attributes the slot's next cycle — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1616
• renderable_candidate is asserted unconditionally and never revised on failure — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1620
• The tombstone outlives its attempt by 270s, which will become attribution noise once the blockers are fixed — see inline at crates/trusted-server-js/lib/src/integrations/gpt/index.ts:1637
• The new Rust test asserts substrings, not behaviour — see inline at crates/trusted-server-core/src/integrations/aps.rs:3325

Cross-cutting / body-level findings

• 🤔 Three of the serious findings share one root cause: store APIs used for something other than what their contracts mean. recordTrustedServerOpportunity means "Trusted Server supplied this slot's next GPT request"; the bridge uses it to mean "APS is about to render into this div". recordTrustedServerCreativeResponse means "this render succeeded"; the bridge uses it to mean "the descriptor was handed off". And the failure vocabulary lives in two places (types.ts union, store.ts validator) that can drift silently — which is exactly what happened. Rather than adding more call sites to the existing surface, consider a narrow APS-shaped store API: one that populates trustedServerSlots without planting a request intent, and one that lets late evidence downgrade a completed attempt. Deriving isCreativeFailure from a single const array would make the union and the validator incapable of drifting again.

• 📝 Fix ordering matters between two of the blockers. The missing source check on the relay branch is currently unexploitable only because every aps_* reason is discarded before it is recorded. Fixing the allowlist without adding the source check in the same change would activate it.

CI Status

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (axum native): PASS
• cargo test (cloudflare native + wasm32-unknown-unknown): PASS — reported as cargo check (cloudflare native + wasm32-unknown-unknown)
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native): PASS
• vitest: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• prepare integration artifacts: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (javascript-typescript): PASS
• Analyze (actions): PASS
• CLAUDE.md symlink guard: PASS

All checks pass. Note that tsc --noEmit is not among them: this branch introduces two new type errors (gpt/index.ts:1613 TS2339, overlay.ts:177 TS2366) that are absent at the merge base.

[aram356]

🔧 wrench — The store's runtime allowlist rejects all fifteen new reasons, so nothing this PR adds is ever recorded.

This union is the compile-time contract. The runtime contract is isCreativeFailure in crates/trusted-server-js/lib/src/integrations/gpt_diagnostics/store.ts:168-175, which this PR does not touch and which still lists only the original four:

function isCreativeFailure(reason: unknown): reason is GptDiagnosticsCreativeFailure {
  return (
    reason === 'missing_render_source' ||
    reason === 'cache_fetch_failed' ||
    reason === 'invalid_cache_payload' ||
    reason === 'response_post_failed'
  );
}

recordTrustedServerCreativeFailure bails on it at store.ts:540 before recording anything — no failure, and not even an attribution issue to signal the drop.

I verified this against the real GptDiagnosticsStore on a live attempt (no response recorded, well inside the attempt window), so nothing else is masking it:

recordTrustedServerCreativeFailure(attemptId, 'aps_frame_timeout')   -> snapshot has no 'aps_frame_timeout'
recordTrustedServerCreativeFailure(attemptId, 'response_post_failed') -> snapshot has 'response_post_failed'

Exhaustively over the union, the four originals are accepted and all fifteen aps_* members are dropped. That makes every new emission site in this PR a no-op: the bridge guards (aps_consumed_tombstone, aps_source_not_in_ad_unit, aps_descriptor_fields, aps_tombstone_capacity, aps_missing_renderer_url) and the entire relay branch. The only reason that still lands is response_post_failed, which already worked before this PR.

Apply manually — the fix lives in store.ts, which this PR does not modify. Rather than hand-extending the validator, derive both from one source so they cannot drift again:

const CREATIVE_FAILURES = [
  'missing_render_source',
  'cache_fetch_failed',
  'invalid_cache_payload',
  'response_post_failed',
  'aps_bad_hash',
  // ... the remaining aps_* members
] as const;

export type GptDiagnosticsCreativeFailure = (typeof CREATIVE_FAILURES)[number];

const CREATIVE_FAILURE_SET: ReadonlySet<string> = new Set(CREATIVE_FAILURES);

function isCreativeFailure(reason: unknown): reason is GptDiagnosticsCreativeFailure {
  return typeof reason === 'string' && CREATIVE_FAILURE_SET.has(reason);
}

[aram356]

🔧 wrench — Widening this union makes creativeFailureFact non-exhaustive, so ts_console renders undefined.

crates/trusted-server-js/lib/src/integrations/gpt_diagnostics/overlay.ts:175-188 switches over this union with no default, declared : string. It is exhaustive on main; this PR adds fifteen members that fall through and return undefined, which overlay.ts:262 pushes straight into the facts list:

facts.push(creativeFailureFact(failure));

Confirmed with tsc --noEmit: overlay.ts(177,4): error TS2366: Function lacks ending return statement and return type does not include 'undefined'. I checked the merge base (066ea3c69) and this error is not present there, so it is introduced by this branch. It does not fail CI because no check runs tsc --noEmit — vitest typechecking covers test files only.

Apply manually — the fix is in overlay.ts, outside this PR's diff. Add the aps_* cases with operator-readable text. Keeping the switch exhaustive with no default is the right call: it is what would have caught this at compile time.

[aram356]

🔧 wrench — The renderer document's new three-key failure message is silently ignored by the direct render path. This is a behavioural regression, not a diagnostics gap.

report() in aps.rs now always posts three keys:

parent.postMessage({message:'trusted-server/aps/renderer-failed',nonce:nonce,reason:reason},'*');

There are two consumers of that message. The Universal Creative source right here was updated to a permissive per-field match. The direct path was not — render.ts:724-731 still gates on an exact two-key match:

if (event.source !== iframe.contentWindow || !hasExactKeys(event.data, ['message', 'nonce'])) {
  return;
}

hasExactKeys compares key counts (render.ts:194-205), so the added reason key fails the match and every renderer-failed message is dropped. fail() never runs.

Reproduced against the real renderApsCreative with a scratch vitest:

{message, nonce}                  -> iframe torn down       (passes, pre-PR behaviour)
{message, nonce, reason}          -> iframe still connected (fails)

This path is live: core/request.ts:59 wires renderApsCreative as the trustedServer renderer. The consequence is that a rejected descriptor or an Amazon script error no longer fails fast — the hidden frame stays mounted over publisher content for the full 10s RENDERER_READY_TIMEOUT_MS, and the log.warn('APS renderer: frame load failed') diagnostic is delayed by the same amount.

Apply manually — the fix is at render.ts:724-731, outside this PR's hunks. This version also handles bad_hash, where the frame never learned a nonce to echo:

function receive(event: MessageEvent): void {
  if (event.source !== iframe.contentWindow || !isRecord(event.data)) return;
  const { message, nonce: sent } = event.data;
  if (message === RENDERER_READY_MESSAGE) {
    if (hasExactKeys(event.data, ['message', 'nonce']) && sent === nonce) commit();
    return;
  }
  // The renderer document attaches a `reason` to its failure message, and
  // reports `bad_hash` before it can echo a nonce.
  if (message === RENDERER_FAILED_MESSAGE && (sent === nonce || sent === undefined)) fail();
}

Scratch-verified in an isolated worktree: teardown on a three-key failure, teardown on nonce-less bad_hash, still ignores a foreign nonce, still rejects a ready message with an extra key. 235 tests pass and prettier is clean. Please add a test asserting the three-key failure tears the frame down — nothing currently covers it.

[aram356]

🔧 wrench — Recording the response here closes the attempt, so every relayed frame failure is dropped even after the allowlist is fixed.

recordTrustedServerCreativeResponse makes the attempt terminal (store.ts:529-531):

attempt.cycle.trustedServerCreativeResponseAtMs ??= timestampMs;
attempt.status = 'completed';
attempt.cycle = undefined;

and recordTrustedServerCreativeFailure returns early on a completed attempt (store.ts:549), silently — no attribution issue either.

This fires synchronously the moment port.postMessage returns, which is when the descriptor is handed off, not when a pixel is drawn. Every relayed failure arrives strictly later: frame_timeout at 10s (render.ts:779), amazon_script_error after the script 404s. So they are all dropped.

Verified against the real store:

recordTrustedServerCreativeRequest -> recordTrustedServerCreativeResponse -> recordTrustedServerCreativeFailure(aps_frame_timeout)

delivery          = trusted_server_response_sent
failures          = undefined
attributionIssues = []

Control: recording the failure before the response yields delivery = trusted_server_selected, so the ordering is genuinely load-bearing.

That result is the exact ambiguity this PR exists to remove. A slot that renders blank reports a clean confirmed delivery — worse than the pre-PR unknown, because it converts "we do not know" into a confident wrong answer.

Apply manually — needs a store-side lifecycle change. Two options: move the response recording to a genuine render-confirmed signal (the renderer-ready acknowledgement rather than the descriptor post), or give the store a post-completion failure state so late evidence can downgrade a completed attempt. The first is closer to what recordTrustedServerCreativeResponse already means.

[aram356]

🔧 wrench — This branch has no test, and the way the new tests are written is why the two blockers in this review ship with green CI.

Two distinct gaps.

The relay branch is uncovered. It is the one place a cross-origin reason becomes a recorded attempt — the untrusted-input boundary of the whole feature. render.test.ts covers the emission side (relay of a frame reason, frame timeout) and apsRenderFailureReason in isolation, but nothing feeds an APS_RENDER_FAILED_MESSAGE into installTsRenderBridge's listener. Untested: the tombstone lookup, the attemptId resolution, the missing-tombstone case, the rejected-reason case, the non-string adId case, and the absent source check.

The new bridge tests cannot catch a store-side gap, by construction. Both tests in ad_init.test.ts:3469-3555 replace gptDiagnosticsRecorder wholesale with vi.fn() spies and then assert the bridge called them:

expect(recordTrustedServerCreativeFailure).toHaveBeenCalledWith(11, 'aps_consumed_tombstone');

That verifies the call site and nothing downstream. Against the real store that call is a no-op. store.test.ts is untouched by this PR and has no aps_* cases, so the gap is covered nowhere.

Apply manually — needs a new test. One end-to-end test wiring installTsRenderBridge to a real GptDiagnosticsStore and asserting on snapshot() would have failed on every new category, and would have caught both blockers. Worth having that test in addition to the mock-based ones, not instead of them.

[aram356]

♻️ refactor — Untyped window access; every other call site in this file casts to GptWindow.

See :954 ((window as GptWindow).googletag), and :320 / :413 / :629 / :821. This one is bare, which produces a new tsc error:

src/integrations/gpt/index.ts(1613,27): error TS2339: Property 'googletag' does not exist on type 'Window & typeof globalThis'.

I confirmed this error is absent at the merge base (066ea3c69), so the branch introduces it. CI does not catch it because no check runs tsc --noEmit over src/.

Suggested change

	const pubads = window.googletag?.pubads?.();
	const pubads = (window as GptWindow).googletag?.pubads?.();

Scratch-verified in an isolated worktree: the TS2339 is gone, prettier is clean, and the 127 ad_init.test.ts tests pass.

[aram356]

🤔 thinking — This stamps the slot's next request cycle as Trusted Server-driven, not the one being rendered.

recordTrustedServerOpportunity does not annotate the current cycle. It writes a pending intent via recordRequestIntentSource (store.ts:359, :934-956) which is consumed by the next slotRequested callback (store.ts:594-596). Meanwhile recordTrustedServerCreativeRequest, called immediately after, resolves against the slot's latest already-recorded cycle. The two calls target different cycles.

Measured against the real store, within the 5s REQUEST_PATH_ATTRIBUTION_WINDOW_MS:

baseline next cycle  -> requestPath = unattributed,         opportunity = undefined, delivery = unknown
polluted next cycle  -> requestPath = trusted_server_direct, opportunity = renderable_candidate, delivery = pending
publisher_refresh + injected opportunity -> requestPath = competing

The third case is the one that concerns me most: a genuine publisher_refresh cycle is relabelled competing because two sources now claim it, which actively destroys correct attribution rather than merely adding noise. And this is the normal path, not an attacker path — every APS handshake on a div that shares an element ID with a GPT slot does it.

The doc comment explains the real constraint well (the store rejects the attempt as creative_request_without_slot without an association), so the intent is clearly right. The issue is that recordTrustedServerOpportunity is the wrong tool: its contract is "Trusted Server supplied this slot's next GPT request", which is not what is being expressed here. What is needed is a narrower store API that populates trustedServerSlots without planting a request intent.

[aram356]

📝 note — renderable_candidate is asserted before any render is attempted and never revised when the render fails.

On its own this is defensible optimism. Combined with the two blockers it produces the worst possible output: a slot that renders blank reports trustedServerOpportunity: 'renderable_candidate', delivery: 'trusted_server_response_sent', and no failures — indistinguishable from a healthy delivery.

Worth considering whether a later failure should downgrade the opportunity, so the record self-corrects once the render outcome is actually known. That is the same lifecycle gap as the completed-attempt finding and would likely be fixed by the same change.

[aram356]

🤔 thinking — The tombstone outlives its attempt by roughly ten attempt windows, which will become attribution noise once the blockers are fixed.

attemptId is retained for the tombstone's full lifetime, but the two clocks differ by an order of magnitude:

• tombstone expiresAt = the renderer entry TTL, DEFAULT_PREBID_RENDERER_TTL_SECONDS = 300 (render.ts:94), up to 3600s
• attempt expiresAtMs = requestedAtMs + CREATIVE_ATTEMPT_WINDOW_MS, and CREATIVE_ATTEMPT_WINDOW_MS = 30_000 (store.ts:25)

pruneConsumedPrebidApsIds prunes on expiresAt only and has no notion of the attempt window, so consumedPrebidApsIds.get(failedAdId)?.attemptId keeps handing out stale IDs for minutes after the attempt expired.

Today this is invisible: the allowlist rejects the reason before any status is inspected, so measuring a relay at CREATIVE_ATTEMPT_WINDOW_MS + 1 gives empty failures and empty attributionIssues. But once the allowlist and the completed-attempt findings are fixed, the expired branch at store.ts:551-555 becomes live and every replay or relay arriving 30s–300s after the handshake emits creative_attempt_expired into a 128-entry ring buffer. aps_consumed_tombstone replays are exactly the long-tail case, so the noise is likely rather than theoretical.

Clearing attemptId (while keeping expiresAt for the tombstone's actual security purpose) once the attempt window has elapsed would avoid it.

[aram356]

🤔 thinking — This test asserts literal presence in a string, not guard behaviour.

It is a substring test over a &str constant and executes no JavaScript, so it cannot show that any guard actually reports. I checked how weak that is concretely: a stub document that defines report() but never calls it, with the seven reason strings parked in a comment, passes every positive assertion here. A reason literal could be deleted from its return statement and left in a comment and the test stays green.

contains("reason:reason") proves the object literal is spelled that way, not that report is ever invoked. contains("if(reported)return") proves the early return is written, not that reported is ever set or that the guard precedes the post.

The three negative assertions are a different matter and genuinely valuable — !contains("JSON.stringify(renderer)"), !contains("reason:message"), and !contains("event.source.postMessage") are real tripwires against a future edit that would leak descriptor data or answer the sender. Those I would keep as-is.

For the positive half, the JS suite already evaluates a sibling document in jsdom (render.test.ts:838 does window.eval(APS_UNIVERSAL_CREATIVE_RENDERER)), so the same technique is available for APS_RENDERER_DOCUMENT and would let these assertions test the guards rather than the source text. That is also the coverage that would have caught the direct-path regression flagged on render.ts.

I confirmed nothing regressed here: cargo test -p trusted-server-core --target aarch64-apple-darwin aps gives 116 passed; 0 failed.