Rename /__ts/page-bids to /_ts/page-bids - #971
Merged
Merged
Conversation
The SPA re-auction endpoint was the only internal route using a double-underscore prefix; every other internal path lives under /_ts/. Move it to /_ts/page-bids and switch the tsjs client fetch to match. A browser runs whichever tsjs bundle it was already served, so pages loaded before the rename — and cached bundles — keep requesting the old path. On a SPA that path delivers ads for in-session navigations, so /__ts/page-bids stays registered to the same handler as a transition alias until those bundles age out. Both paths are defined once in core as PAGE_BIDS_PATH and PAGE_BIDS_LEGACY_PATH so removal touches one const plus its four registrations. handle_page_bids logs an info line when the alias serves a gate-passed request. Without it nothing in the app distinguishes the two paths, so the "no remaining legacy traffic" precondition for removing the alias would only be answerable from edge access logs. Route coverage was missing for the page-bids GET registrations on Cloudflare and Spin, where GET and OPTIONS are registered separately and the preflight-denial parity test does not imply the GET side is wired. Mutation testing confirmed a broken registration previously passed every suite. The route-table assertions pin literal paths rather than the consts, since looking a route up by the same const it was registered with still passes when the const's value changes.
ChristianPavilonis
approved these changes
Jul 28, 2026
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Summary
Reviewed the route rename and transition alias across core, all four adapters, the TSJS client, rollout behavior, and test coverage. The routing and security-gate implementation is otherwise consistent, and all CI checks pass. Approving with one high-severity compatibility finding that should be addressed for deployments using broad operator-defined path authentication rules.
aram356
approved these changes
Jul 29, 2026
aram356
approved these changes
Jul 29, 2026
Address review findings on the /__ts/page-bids to /_ts/page-bids rename. The rename moved a browser-facing endpoint into the namespace operators protect with [[handlers]] Basic Auth. A pattern broad enough to cover /_ts (rather than ^/_ts/admin) now answers /_ts/page-bids with 401 that no anonymous browser fetch can satisfy, so SPA navigations lose ads. The same failure appears inverted on rollback: a server reverted past the rename does not register the canonical path and proxies it to the publisher origin. The tsjs SPA hook now falls back to the deprecated alias once when the canonical path returns 401/404 or a non-JSON body, then stays on it for the session. 403 (the cross-site gate) and 5xx apply to both paths and do not retry. This is a bridge, not the fix: document the handler-scoping constraint so affected operators narrow their pattern before the alias is removed in #970. Fallback requests send X-TSJS-Page-Bids: fallback so the deprecation signal stays honest. Traffic from a pre-rename bundle ages out on its own and logs at info; a current bundle falling back does not age out, because the cause is deployment configuration, and logs at warn with a config hint. Collapsing the two would make #970 wait forever on a config problem. The gate checks the header's presence, not its value, so the diagnostic does not weaken it. Also on the alias: attach the RFC 9745 deprecation link relation so remaining traffic is countable from edge logs without app-log access, and run the cross-site gate before the not-configured 404 so alias traffic on deployments without creative opportunities is still counted (and so a cross-site caller cannot probe that configuration). RFC 9745's companion Deprecation field is omitted: it carries a date, and there is no source of truth here for when the alias was deprecated.
prk-Jr
added a commit
that referenced
this pull request
Jul 29, 2026
rc/july merged the 942-page-bids-single-underscore branch before its second review round; the #971 squash on main includes that round. This merge is a union, not a fast-forward of either side. Resolutions: - publisher.rs takes main's round-two work — PAGE_BIDS_FALLBACK_MARKER, the warn/info split that separates a current bundle falling back from a pre-rename bundle, mark_deprecated_alias on both the 404 and success paths, and the cross-site gate moved above the not-configured 404 so a cross-site caller cannot probe whether creative opportunities are configured. Keeps rc/july's normalize_page_bids_path_and_query, trace_enabled, and request origin. Main's raw query binding is renamed requested_page to feed rc/july's normalize pipeline. - gpt/index.ts takes main's fetchPageBids/requestPageBids alias fallback and keeps rc/july's lastAppliedPath tracking pathname and search. - spa_hook.test.ts keeps rc/july's path assertion, which carries the query string; main's assertion predates that change. - telemetry.rs keeps rc/july's side: AuctionSource moved to types.rs, where the renamed path doc comment already lives. - orchestrator.rs keeps rc/july's side; main's only change there was two path comment renames rc/july already had. - CHANGELOG.md keeps both sets of entries.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/__ts/page-bidswas the only internal Trusted Server route using a double-underscore prefix; every other internal path lives under/_ts/. This moves the SPA re-auction endpoint to/_ts/page-bidsand switches the tsjs client fetch to match./__ts/page-bidsstays registered to the same handler as a transition alias. Removal is tracked by Remove the deprecated /__ts/page-bids transition alias #970.GETregistrations on Cloudflare and Spin. Mutation testing confirmed a broken registration previously passed every suite; that gap is now closed.Changes
crates/trusted-server-core/src/publisher.rsPAGE_BIDS_PATH/PAGE_BIDS_LEGACY_PATHas the single source of truth; log aninfoline when the alias serves a gate-passed request; adddeprecated_alias_response_matches_canonical_path; splitmake_page_bids_request_onout ofmake_page_bids_request; update doc commentscrates/trusted-server-adapter-fastly/src/app.rsNAMED_ROUTES; addpage_bids_serves_canonical_path_and_deprecated_alias, pinning the consts to their literal valuescrates/trusted-server-adapter-axum/src/app.rsnamed_routes()widens to[NamedRoute; 13]crates/trusted-server-adapter-cloudflare/src/app.rslegacy_admin_denypatterncrates/trusted-server-adapter-spin/src/app.rsnamed_fallback_paths()widens to 13 so non-primary methods still reach the publisher fallbackcrates/trusted-server-adapter-axum/tests/routes.rsGET/OPTIONSon both paths inall_explicit_routes_are_registeredcrates/trusted-server-adapter-cloudflare/tests/routes.rscrates/trusted-server-adapter-spin/tests/routes.rspage_bids_get_is_routed_on_canonical_path_and_alias(Spin has no route-list test)crates/trusted-server-integration-tests/tests/parity.rscrates/trusted-server-js/lib/src/integrations/gpt/index.ts/_ts/page-bidscrates/trusted-server-js/lib/test/integrations/gpt/spa_hook.test.tscrates/trusted-server-core/src/auction/{endpoints,orchestrator,telemetry}.rs,integrations/gpt.rsCloses
Closes #942
Test plan
cargo test-fastly && cargo test-axum— alsocargo test-cloudflare && cargo test-spincargo clippy-fastly && cargo clippy-axum— alsoclippy-cloudflare,clippy-cloudflare-wasm,clippy-spin-native,clippy-spin-wasmcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest run(411 passed)cd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serve--test parity, 13 passed); mutation testing of all 8 registrationsMutation testing
Because the alias exists precisely to avoid a silent ad-serving gap, each registration was deliberately broken to confirm a test fails. All 8 are caught (Fastly / Axum / Cloudflare / Spin × canonical / alias). Before this PR's added coverage, breaking the Cloudflare
GETregistration — canonical or alias — passed every suite.Verified the built bundle rather than assuming:
dist/is gitignored and regenerated bybuild.rs; the rebuilttsjs-gpt.jscontains/_ts/page-bidsonce and/__ts/zero times. The separately-shipped external Prebid bundle takes onlyprebid/index.tsand does not embed this path, so there is no half-ship risk.Also confirmed the namespace move does not change behaviour: every handler regex in the repo is
^/_ts/admin, which does not match/_ts/page-bids, so the route is not pulled behind basic auth. Nostarts_with("/_ts")special-casing exists; integration path filters are all under/integrations/; cache and privacy logic key on headers, never path.Notes
The
docs/superpowers/plan and spec files still reference/__ts/page-bids. They are dated point-in-time design records that also describe code shapes since changed, so they were left as history rather than rewritten. Happy to sweep them if reviewers prefer.Not applicable: no config-derived regex or pattern compilation is touched.
Checklist
unwrap()in production code — useexpect("should ...")logmacros (notprintln!)