Sync EdgeZero PR #257 updates#761
Conversation
f078d3b to
396d270
Compare
41e7268 to
b6c4a79
Compare
b6c4a79 to
159c731
Compare
Point the edgezero dependencies at the upstream main branch and bump fastly/log-fastly to 0.12 to match edgezero's pinned version. Forward-port the body and TLS APIs to the newer surface: - Body::into_bytes() now returns Option<Bytes>; buffered-body call sites use unwrap_or_default() to preserve prior semantics. - fastly 0.12 get_tls_protocol()/get_tls_cipher_openssl_name() return Result<Option<&str>>; adapter call sites use .ok().flatten(). Resync the excluded integration-tests lockfile to the same versions.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed PR #761 against main, focusing on the EdgeZero/Fastly API update, crate/path renames, build and CI wiring, and the touched runtime code paths. CI is green and I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues. I left one low-severity documentation/operational comment.
The wrapper script lives at crates/trusted-server-openrtb/generate.sh, not in the codegen crate. Point the example at the actual script path.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Automated review: I reviewed PR #761 against main, focusing on the EdgeZero/Fastly API migration, Body/TLS API call sites, crate renames, CI/script path updates, lockfiles, OpenRTB regeneration paths, and touched runtime/security-sensitive code. CI is green, existing CodeQL/README feedback has already been addressed or is non-blocking, and I did not find any blocking correctness, security, data-loss, authorization, or severe compatibility issues in this pass.
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Review Summary
😃 Approved. I reviewed PR #761 against main, with another pass over the rename/tooling/doc fallout after the EdgeZero/Fastly sync. CI is green.
🔧 I left a few non-blocking inline doc suggestions using GitHub suggestion blocks so they can be applied directly.
📝 One additional cleanup is worth doing but could not be attached as an inline suggestion because those files are not part of the PR diff: tracked Claude helper files still reference the deleted crates/js/lib path (for example .claude/commands/check-ci.md, .claude/commands/verify.md, .claude/agents/verify-app.md, .claude/agents/build-validator.md). Please replace remaining crates/js references with crates/trusted-server-js so local/agent verification helpers keep working after the crate rename.
| ### 7. Provide Static Assets (If Needed) | ||
|
|
||
| Place any integration-specific JavaScript entrypoint under `crates/js/lib/src/integrations/` (for example, `crates/js/lib/src/integrations/testlight.ts`). The shared `npm run build` script automatically discovers every file in that directory and produces a bundle named `tsjs-<entry>.js`, which the Rust crate embeds as `/static/tsjs=tsjs-<entry>.min.js`. | ||
| Place any integration-specific JavaScript entrypoint under `crates/trusted-server-js/lib/src/integrations/` (for example, `crates/trusted-server-js/lib/src/integrations/testlight.ts`). The shared `npm run build` script automatically discovers every file in that directory and produces a bundle named `tsjs-<entry>.js`, which the Rust crate embeds as `/static/tsjs=tsjs-<entry>.min.js`. |
There was a problem hiding this comment.
🔧 Docs: match the actual JS entrypoint discovery shape
The build discovers integration directories with an index.ts entrypoint (crates/trusted-server-js/lib/build-all.mjs), not arbitrary .ts files directly under src/integrations. Following the current example would create testlight.ts, which is not bundled.
| Place any integration-specific JavaScript entrypoint under `crates/trusted-server-js/lib/src/integrations/` (for example, `crates/trusted-server-js/lib/src/integrations/testlight.ts`). The shared `npm run build` script automatically discovers every file in that directory and produces a bundle named `tsjs-<entry>.js`, which the Rust crate embeds as `/static/tsjs=tsjs-<entry>.min.js`. | |
| Place any integration-specific JavaScript entrypoint under `crates/trusted-server-js/lib/src/integrations/<integration-id>/index.ts` (for example, `crates/trusted-server-js/lib/src/integrations/testlight/index.ts`). The shared `npm run build` script automatically discovers every integration directory with an `index.ts` file and produces a bundle named `tsjs-<integration-id>.js`, which the Rust crate embeds as `/static/tsjs=tsjs-<integration-id>.min.js`. |
|
|
||
| - `crates/trusted-server-core/src/integrations/testlight.rs` - Rust implementation | ||
| - `crates/js/lib/src/integrations/testlight.ts` - TypeScript shim | ||
| - `crates/trusted-server-js/lib/src/integrations/testlight.ts` - TypeScript shim |
There was a problem hiding this comment.
🔧 Docs: point at the real Testlight TS entrypoint
testlight.ts does not exist after the directory-entrypoint build layout; the actual bundle entry is testlight/index.ts.
| - `crates/trusted-server-js/lib/src/integrations/testlight.ts` - TypeScript shim | |
| - `crates/trusted-server-js/lib/src/integrations/testlight/index.ts` - TypeScript shim |
| ### Bundle Types | ||
|
|
||
| Each integration is built as a separate IIFE at compile time (`crates/js/lib/dist/`): | ||
| Each integration is built as a separate IIFE at compile time (`crates/trusted-server-js/lib/dist/`): |
There was a problem hiding this comment.
🔧 Docs: fix the generated bundle output directory
build-all.mjs writes to the crate-level dist directory (crates/trusted-server-js/dist), not lib/dist.
| Each integration is built as a separate IIFE at compile time (`crates/trusted-server-js/lib/dist/`): | |
| Each integration is built as a separate IIFE at compile time (`crates/trusted-server-js/dist/`): |
Summary
Syncs this branch with the EdgeZero tooling/dependency updates and standardizes
crate naming, rebased on top of
main. Since this branch was opened,mainlanded its own HTTP-types migration (PR 12 / PR 13 — #624, #626) and additional
features (DataDome server-side protection #769, configurable integration proxy
paths #759, image proxy fixes, and several test-coverage additions). The
overlapping work has been reconciled in favor of
main's newer architecture,so this PR now carries only the changes that are genuinely additive over
main.EdgeZero dependency + toolchain
edgezero-*git dependencies at the upstreammainbranch(rather than a pinned rev), so this app tracks current EdgeZero.
fastly/log-fastlyto 0.12 to match the version EdgeZeromainpins.
edgezero_core::body::Body::into_bytes()now returnsOption<Bytes>(
Noneonly for streaming bodies). Buffered-body call sites use.unwrap_or_default(), preserving priorBytessemantics. Forsize-bounded reads,
Body::into_bytes_bounded(max)is now available.get_tls_protocol()/get_tls_cipher_openssl_name()nowreturn
Result<Option<&str>>; adapter call sites use.ok().flatten().1.91.1 → 1.95.0(rust-toolchain.toml+.tool-versions), Fastly CLI13.3.0 → 15.1.0, Viceroy0.16.4 → 0.17.0,add Wasmtime
44.0.1.Crate naming
trusted-server-*prefix:crates/js→crates/trusted-server-jscrates/openrtb→crates/trusted-server-openrtbcrates/openrtb-codegen→crates/trusted-server-openrtb-codegencrates/integration-tests→crates/trusted-server-integration-testsclippy.tomlto the EdgeZero-style configuration.Reconciliation decisions (vs. the original PR intent)
main's HTTP-types migration supersedes thisbranch's older
fastly::-based integration code, so the conflicted files takemain's versions (thehttp::/ async EdgeZero platform surface).restriction = "deny"workspace posture. That produced large, unrelated churnagainst
main's current code, so the workspace lint levels stay atmain's posture. (clippy.tomlconfig is still refreshed;clippy -D warningsis green.)Cargo.lockand the excludedtrusted-server-integration-tests/Cargo.lockare resynced to EdgeZeromain--locked.Known limitation (tracked separately)
trusted-server-coreis not yet platform-agnostic: it still has anunconditional
fastlydependency, with the coupling incompat.rs(afastly::Request↔http::Requestbridge) andbackend.rs(
fastly::backend::Backend). Onlytrusted-server-adapter-fastlyexists today;the workspace declares (currently unused)
edgezero-adapter-axum/edgezero-adapter-cloudflare, and there is no Spin adapter. Extracting theFastly coupling out of core so it can run on the other EdgeZero adapters
(Cloudflare Workers, Spin, Axum) is intentionally out of scope for this PR
and will be done separately.
Verification
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspacecargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1cargo metadata --locked(root)cargo metadata --locked(crates/trusted-server-integration-tests)cargo metadata --locked(crates/trusted-server-openrtb-codegen)cd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatReferences stackpop/edgezero#257