[dhruv8sh]

Summary

• Plan-registered integrations (Prebid, APS) were added to the registry before the builder table, so Prebid's attribute rewriter ran ahead of js_asset_proxy and removed publisher Prebid.js tags that an operator had explicitly mapped with proxy = "enabled". The registry now stably sorts the js_asset_proxy rewriter first; other rewriters keep registration order.
• A js_asset_proxy replacement is now terminal: rewrite_attribute returns it without passing it to later rewriters. Without this, an enabled asset whose first-party path matches Prebid script_patterns (for example /assets/prebid.min.js) was removed by Prebid after the rewrite. Other integrations' replacements still chain.
• An enabled js_asset_proxy asset whose origin_url matches Prebid script_patterns would load a second Prebid.js next to the managed bundle, so settings validation now rejects it, listing every conflicting asset, with an actionable message (use disabled, blocked, or drop the pattern).

Upgrade impact

The overlap check runs in both validate_settings_for_deploy and validate_settings_for_runtime, per review: invalid enabled configuration fails startup rather than being logged and skipped. A config already deployed with this contradiction will fail to start after upgrade. CHANGELOG.md has a Breaking entry telling operators to run ts config validate before upgrading, and the configuration, CLI and Prebid guides document the rule.

Changes

File	Change
crates/trusted-server-core/src/integrations/registry.rs	Stable-sort js_asset_proxy attribute rewriter first in with_plan; make its replacement terminal in rewrite_attribute; add test-only attribute_rewriter_ids()
crates/trusted-server-core/src/integrations/mod.rs	Update comment: precedence is enforced by the registry, not list position
crates/trusted-server-core/src/integrations/prebid.rs	Expose matches_script_url (pub(crate)) for reuse by config validation
crates/trusted-server-core/src/config.rs	Add validate_js_asset_proxy_prebid_overlap reporting all conflicts; tests for reject (enabled, multiple conflicts) and accept (disabled/blocked, Prebid disabled, js_asset_proxy disabled)
crates/trusted-server-core/src/integrations/js_asset_proxy.rs	HTML tests: enabled rewrites before Prebid removal; a replacement whose path matches Prebid patterns is kept; blocked and disabled remove the script
crates/trusted-server-core/src/auction/mod.rs	Test: with an APS plan, js_asset_proxy is first and other rewriters keep order; shared aps_provider_map fixture
CHANGELOG.md, docs/guide/configuration.md, docs/guide/cli.md, docs/guide/integrations/prebid.md	Breaking entry and operator docs for precedence and the overlap rule

Closes

Closes #1205

Spec

Not a feature

Test plan

• cargo test-fastly && cargo test-axum
• cargo clippy-fastly && cargo clippy-axum
• cargo fmt --all -- --check
• JS tests: cd crates/trusted-server-js/lib && npx vitest run (1838 passed / 1 skipped, Node 24.20.0)
• JS format: cd crates/trusted-server-js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: all 8 clippy aliases, test-fastly-reuse, test-cloudflare, test-spin, CLI tests, template_build_digest, parity tests

Checklist

• Feature PRs: spec linked above, ending in a review log of two or more agent review rounds (N/A)
• Changes follow AGENTS.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses log macros (not println!)
• New code has tests
• No secrets or credentials committed

[ChristianPavilonis]

Summary

The stable sort restores js_asset_proxy's execution order and the overlap validation makes contradictory Prebid configuration explicit. However, downstream rewriting can still remove a validated proxy replacement, and the new upgrade-time startup rejection needs operator-facing migration guidance.

Blocking

🔧 wrench

• Make explicit proxy replacements terminal — see inline at crates/trusted-server-core/src/integrations/registry.rs:928–931.

Cross-cutting / body-level findings

🔧 Document the upgrade-time startup rejection

The new rejection at crates/trusted-server-core/src/config.rs:360–366 applies to runtime startup as well as deploy validation. Previously accepted configurations can therefore prevent an upgraded server from starting. Strict validation is reasonable, but the migration warning currently exists only in the PR description, not in the operator-facing release notes.

Add this under CHANGELOG.md → Unreleased → Changed:

- **Breaking:** With `[integrations.prebid]` enabled, JS asset entries with
  `proxy = "enabled"` whose `origin_url` matches Prebid `script_patterns`
  now fail deploy validation and runtime startup. Before upgrading the
  server, use the new `ts` CLI to run `ts config validate`, change overlapping
  assets to `proxy = "disabled"` (preserves Prebid interception) or
  `proxy = "blocked"`, and publish the corrected config with `ts config push`.

Apply manually — CHANGELOG.md is outside this PR's diff, so this cannot be auto-applied as a suggestion.

Local Verification

• The destination-path regression reproducer fails on the reviewed head and passes with the original rewriter order.
• The proposed terminal-replacement fix passes 68 targeted native core tests and cargo fmt --all -- --check.
• The full cross-adapter gates were not rerun for the proposed fix.
• All scratch changes were discarded; the reviewer worktree is clean at the reviewed head.

CI Status

• integration tests (Fastly EC lifecycle): PASS
• trace runtime and browser acceptance: PASS
• browser integration tests: PASS
• integration tests: PASS
• CodeQL: PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• vitest: PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• ts dev lint domains (changed lines): PASS
• worker-deadlines (cloudflare): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• worker-deadlines (spin): PASS
• docs-proposal scripts: PASS
• Analyze (javascript-typescript): PASS
• format-docs: PASS (required)
• cargo test (axum native): PASS
• HTML EOF runtime (Fastly): PASS
• format-typescript: PASS (required)
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• CI Node pin guard: PASS
• cargo fmt: PASS (required)
• Analyze (rust): PASS
• Analyze (actions): PASS
• cargo test (cross-adapter parity): PASS
• prepare integration artifacts: PASS
• cargo test: PASS (required)
• CLAUDE.md symlink guard: PASS
• Analyze (python): PASS
• Analyze (javascript-typescript): PASS

[aram356]

Summary

Sorting js_asset_proxy first and rejecting enabled assets that Prebid intercepts are the right pieces, and the validator reuses the runtime matcher (PrebidIntegrationConfig::matches_script_url), so validation cannot drift from what the page does. The sort still breaks a previously working mapping, the new startup rejection is not in the operator docs or CHANGELOG, and a few test and message gaps remain.

4 of the inline comments below carry a one-click GitHub suggestion; use Commit suggestion (or Add suggestion to batch for several at once) to apply them as commits on the PR branch. The remaining inline comment describes its fix in prose because it touches code outside the diff, and the three body-level findings below touch files outside the diff.

Blocking

🔧 wrench

• Pin the disabled-integration guard with a test: see inline at crates/trusted-server-core/src/config.rs:1688
• Report every conflicting asset, not just the first: see inline at crates/trusted-server-core/src/config.rs:354-365
• Say that validation rejects this fixture: see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:685
• Doc comment is narrower than the behavior: see inline at crates/trusted-server-core/src/integrations/prebid.rs:590
• Share the APS provider fixture: see inline at crates/trusted-server-core/src/auction/mod.rs:239-250
• Make js_asset_proxy replacements terminal, CHANGELOG entry for the startup rejection, Document the new rule in the operator guides: see below

Cross-cutting / body-level findings

• 🔧 Make js_asset_proxy replacements terminal (concur with the existing thread at registry.rs:931). Reproduced independently. With path = "/assets/prebid.min.js", origin_url = "https://cdn.example.com/vendor.js" and proxy = "enabled", the base rewriter order (["prebid", "js_asset_proxy"]) keeps <script src="/assets/prebid.min.js"></script>, while this head (["js_asset_proxy", "prebid"]) outputs <html><body></body></html>. Both validate_settings_for_deploy and validate_settings_for_runtime accept that config. Two further points:

  • In any config that passes the new validation, the sort changes Prebid's outcome in exactly one case, and it is this regression: the case the sort targets (an enabled asset whose origin_url Prebid matches) is now rejected at startup. The sort's benefit today is only protection against URL-replacing rewriters that a future change registers ahead of js_asset_proxy.
  • validate_js_asset_proxy_prebid_overlap says an intercepted enabled asset "is either a dead entry or loads a second Prebid.js copy". Under the new order, an enabled asset whose path matches Prebid's patterns is exactly such a dead entry, and it passes validation.

  The terminal-replacement change proposed in that thread fixes both: with it applied, the reproducer keeps the proxied script and all 3348 native trusted-server-core lib tests pass. If you keep chaining instead, the validator also has to reject enabled assets whose path matches script_patterns.

• 🔧 CHANGELOG entry for the startup rejection (concur with the existing review), and an answer to "Decision for reviewers". Keep the check in validate_settings_for_runtime: the project rule is that invalid enabled configuration fails startup instead of being logged and skipped, and a deploy-only check would leave never-revalidated configs loading two Prebid.js copies. That makes the Breaking entry under CHANGELOG.md → Unreleased → Changed required, since a previously accepted config stops the upgraded server from starting.

• 🔧 Document the new rule in the operator guides. docs/guide/cli.md:337-343 tells operators to change audit-drafted entries to "enabled"; ts audit generate marks Prebid candidates with # Native integration may be preferable: [integrations.prebid], and flipping one whose URL matches script_patterns now fails ts config validate and startup. Neither the JS Asset Proxy section in docs/guide/configuration.md nor the Prebid guide's Script Interception section mentions the rule or where precedence is enforced. Proposed text (checked with prettier --config docs/.prettierrc --check):

  docs/guide/configuration.md, after the per-asset table (line 1878). The first paragraph assumes the terminal-replacement fix above:

  JS Asset Proxy decides first for each script whose `src` matches an asset's
  `origin_url`, before other integrations rewrite the tag: `enabled` serves it
  from `path`, `blocked` removes the script, and `disabled` leaves it to other
  integrations.
  
  While `[integrations.prebid]` is enabled, an `enabled` asset whose `origin_url`
  matches Prebid `script_patterns` fails `ts config validate` and server startup,
  because it would load a second Prebid.js next to the managed bundle. Use
  `proxy = "disabled"` to let Prebid intercept the script, or `proxy = "blocked"`
  to remove it.

  docs/guide/cli.md, as a new paragraph after line 343:

  While `[integrations.prebid]` is enabled, an entry whose URL matches Prebid
  `script_patterns` cannot be `"enabled"`; keep it `"disabled"` so Prebid
  intercepts the script, or set it to `"blocked"`.

  docs/guide/integrations/prebid.md, after the first Script Interception paragraph (line 468):

  A `[[integrations.js_asset_proxy.assets]]` entry for an intercepted URL must use
  `proxy = "disabled"` (Prebid intercepts it) or `proxy = "blocked"`; `enabled`
  fails validation.

  Apply manually: these files are outside this PR's diff, so they cannot be one-click suggestions.

Local verification

• Each of the 4 suggestion blocks was applied alone to a clean checkout of this head and passed cargo fmt --all -- --check, cargo clippy-fastly, cargo check-fastly, cargo check-axum and cargo check-cloudflare, plus its targeted native core tests. A byte-level diff confirmed verification did not change the applied patch.
• All 4 applied together passed cargo fmt --all -- --check, all six adapter clippy aliases (clippy-fastly, clippy-axum, clippy-cloudflare, clippy-cloudflare-wasm, clippy-spin-native, clippy-spin-wasm), cargo test-fastly (3654 passed), cargo test-axum (60), cargo test-cloudflare (57), cargo test-spin (96) and the parity suite (28), with no failures.
• The aps_provider_map helper proposed for auction/mod.rs passes cargo fmt --all -- --check, cargo clippy-fastly and the plan_sharing_tests module.
• The destination-path reproducer and the terminal-replacement probe ran on this head and on this head with the sort removed (base order), as described above.

CI Status

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (axum native): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• HTML EOF runtime (Fastly): PASS
• worker-deadlines (cloudflare): PASS
• worker-deadlines (spin): PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• trace runtime and browser acceptance: PASS
• prepare integration artifacts: PASS
• vitest: PASS
• ts dev lint domains (changed lines): PASS
• docs-proposal scripts: PASS
• CI Node pin guard: PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (actions): PASS
• Analyze (python): PASS
• Analyze (javascript-typescript): PASS (both runs)

[dhruv8sh]

@ChristianPavilonis CHANGELOG: added the Breaking entry under Unreleased → Changed in fdc831f, with a sentence noting that JS Asset Proxy now rewrites first and its enabled replacement is final.

[dhruv8sh]

@aram356 Body-level items:

• Terminal replacement: c1daf4d. With it, an enabled asset whose path matches script_patterns is kept rather than becoming a dead entry, so the validator stays origin_url-only; its doc comment now says so.
• Decision for reviewers: kept the check in validate_settings_for_runtime; Breaking CHANGELOG entry in fdc831f. PR description updated.
• Operator guides: your proposed text added to docs/guide/configuration.md, docs/guide/cli.md and docs/guide/integrations/prebid.md in fdc831f; prettier clean.

[ChristianPavilonis]

Summary

The registry now gives js_asset_proxy deterministic precedence and makes its replacements terminal while preserving replacement chaining for other integrations. The shared Prebid matcher, deploy/runtime overlap validation, regression coverage, and upgrade documentation are consistent; no blocking or non-blocking findings.

Verification

• Four temporary native-core probes passed: URL normalization and suffix/wildcard overlap checks on both validation paths; canonical and protocol-relative HTML sources; exact asset dispatch beneath a Prebid wildcard route; and non-proxy chaining, removal short-circuiting, and terminal proxy replacement.
• git diff --check: PASS.
• Probe code was discarded; the review worktree is clean at the reviewed commit.
• Manual browser/runtime testing was not performed.

CI Status

• cargo test (ts CLI, native) (macos-latest): PASS
• browser integration tests: PASS
• trace runtime and browser acceptance: PASS
• integration tests (Fastly EC lifecycle): PASS
• integration tests: PASS
• CodeQL: PASS
• CI Node pin guard: PASS
• format-typescript: PASS (required)
• worker-deadlines (cloudflare): PASS
• worker-deadlines (spin): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• Analyze (javascript-typescript): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• Analyze (rust): PASS
• cargo fmt: PASS (required)
• cargo test (axum native): PASS
• Analyze (actions): PASS
• vitest: PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo test: PASS (required)
• format-docs: PASS (required)
• CLAUDE.md symlink guard: PASS
• docs-proposal scripts: PASS
• cargo test (cross-adapter parity): PASS
• HTML EOF runtime (Fastly): PASS
• prepare integration artifacts: PASS
• ts dev lint domains (changed lines): PASS
• Analyze (python): PASS
• Analyze (javascript-typescript): PASS

[aram356]

Summary

Every round-1 finding is fixed as proposed, and the round-1 probes and mutations now fail against the new tests. Two items remain: the branch conflicts with main after #917, and the "no later rewrite" half of the terminal-replacement contract has no test.

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 other comment describes its fix in prose because a suggestion cannot resolve a conflict with the base branch.

Blocking

🔧 wrench

• Resolve the CHANGELOG conflict with main: see inline at CHANGELOG.md:12
• Pin both halves of the terminal-replacement contract: see inline at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:717

Local verification

• Round-1 probes on this head: the destination-path mapping (path = "/assets/prebid.min.js", origin_url = "https://cdn.example.com/vendor.js") keeps the proxied script. Removing the sort, removing the terminal return, removing the !config.enabled guard, or reporting only the first conflict each makes a specific new test fail.
• With the suggested test applied to this head: cargo fmt --all -- --check, all six adapter clippy aliases, cargo test-fastly (3659 passed), cargo test-axum (60), cargo test-cloudflare (57), cargo test-spin (96) and the parity suite (28) pass, and verification did not change the posted bytes. The test also passes on the scratch merge with main (3374 native core tests).
• The downstream attribute rewriters (GPT, GTM, DataDome, Sourcepoint, Lockr, Permutive, testlight) match vendor hosts or vendor-specific substrings, so terminal replacement only changes the outcome for an operator-chosen path that one of them matches, which is the case this PR fixes.

CI Status

These runs predate #917; GitHub cannot build a merge ref while the PR conflicts, so they do not cover the merge with current main.

• cargo fmt: PASS (required)
• cargo test: PASS (required)
• format-docs: PASS (required)
• format-typescript: PASS (required)
• cargo test (axum native): PASS
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• cargo test (cross-adapter parity): PASS
• cargo test (ts CLI, native) (ubuntu-latest): PASS
• cargo test (ts CLI, native) (macos-latest): PASS
• HTML EOF runtime (Fastly): PASS
• worker-deadlines (cloudflare): PASS
• worker-deadlines (spin): PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• browser integration tests: PASS
• trace runtime and browser acceptance: PASS
• prepare integration artifacts: PASS
• vitest: PASS
• ts dev lint domains (changed lines): PASS
• docs-proposal scripts: PASS
• CI Node pin guard: PASS
• CLAUDE.md symlink guard: PASS
• CodeQL: PASS
• Analyze (rust): PASS
• Analyze (actions): PASS
• Analyze (python): PASS
• Analyze (javascript-typescript): PASS (both runs)

[prk-Jr]

Summary

The fix matches the design proposed in #1205: js_asset_proxy is sorted first, its enabled replacement is terminal, and the Prebid overlap check rejects the one contradictory config. I found no new defects. The overlap check compares the same URL that asset_for_script_src matches, and Prebid's rewriter is registered only under the same [integrations.prebid] enabled condition the check uses, so the check covers exactly the tags Prebid would remove. Every round-1 finding is addressed in c1daf4d71 and 49992fad6.

Two items from the previous review on this head remain open, and I agree both should land before merge.

Blocking

🔧 wrench

• Resolve the CHANGELOG.md conflict with main: see the existing inline thread at CHANGELOG.md:12. A local git merge-tree of this head with current main (598a85171) conflicts only in CHANGELOG.md; GitHub reports the PR as CONFLICTING, so the green runs do not cover the merge with current main.
• Pin the chaining half of the terminal-replacement contract: see the existing inline thread at crates/trusted-server-core/src/integrations/js_asset_proxy.rs:717. Independently confirmed: changing rewrite_attribute so every integration's Replace returns immediately (no chaining for anyone) leaves the full native trusted-server-core lib suite green (3351 passed). The suggested AppendChainedRewriter test closes that gap.

Local verification

• Targeted native trusted-server-core tests on this head (js_asset_proxy, validation_*, plan_sharing_tests): 104 passed.
• Not run here: the Viceroy Fastly suite, other adapter suites, clippy, or a runtime check.

CI Status

All 29 reported checks PASS on 154667ef3; they predate the conflict with current main.

[dhruv8sh]

Both blocking items are addressed: main is merged (b6f437e) and the chaining test is added (ca49b92). The fmt, clippy (six adapter aliases), adapter test suites and parity gates were run on the merged head.