Make body hold parser-aware and stream Next.js processing by prk-Jr · Pull Request #1135 · IABTechLab/trusted-server

prk-Jr · 2026-09-07T16:32:26Z

Summary

Use a parser-generated, request-specific seam to delay only the structural </body> tail, preventing inline JavaScript or JSON literals from holding most of the page.
Replace whole-document Next.js post-processing with bounded per-document streaming that rewrites complete RSC groups and restores malformed, incomplete, or over-limit payloads unchanged.
Preserve auction ordering and terminal telemetry across streaming, compressed, missing-body-end, and failure paths.

Changes

File	Change
`crates/trusted-server-core/src/html_processor.rs`	Insert a parser-confirmed deferred body marker and stream transformed output.
`crates/trusted-server-core/src/integrations/google_tag_manager.rs`	Adapt registration to the per-document processor interface.
`crates/trusted-server-core/src/integrations/mod.rs`	Define the per-document HTML stream processor factory contract.
`crates/trusted-server-core/src/integrations/nextjs/html_post_process.rs`	Remove the whole-document Next.js post-processor.
`crates/trusted-server-core/src/integrations/nextjs/mod.rs`	Register the bounded Next.js stream processor.
`crates/trusted-server-core/src/integrations/nextjs/rsc.rs`	Classify malformed and incomplete RSC T-chunks safely, including decoded character boundaries.
`crates/trusted-server-core/src/integrations/nextjs/rsc_placeholders.rs`	Capture RSC scripts into request-namespaced placeholders with bounded fallback.
`crates/trusted-server-core/src/integrations/nextjs/rsc_stream.rs`	Add ordered, bounded RSC grouping, rewriting, restoration, and streaming release.
`crates/trusted-server-core/src/integrations/nextjs/script_rewriter.rs`	Move Next.js script fragment state to the document scope.
`crates/trusted-server-core/src/integrations/nextjs/shared.rs`	Share URL rewrite support with the stream processor.
`crates/trusted-server-core/src/integrations/registry.rs`	Construct processor instances per document.
`crates/trusted-server-core/src/publisher.rs`	Collect auctions at the parser-confirmed seam while preserving early output and abandonment telemetry.
`docs/guide/integrations/nextjs.md`	Document bounded Next.js streaming and unchanged fallback behavior.
`docs/superpowers/specs/2026-09-07-850-parser-aware-body-hold-nextjs-streaming-design.md`	Add the reviewed design specification.
`docs/superpowers/plans/2026-09-07-850-parser-aware-body-hold-nextjs-streaming.md`	Add the reviewed implementation plan.

Closes

Closes #850

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 (blocked by the existing CommonJS/ESM incompatibility between html-encoding-sniffer and @exodus/bytes; no JS files changed)
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: cargo test-cloudflare, cargo test-spin, cargo clippy-cloudflare, cargo clippy-cloudflare-wasm, cargo clippy-spin-native, and cargo clippy-spin-wasm

Checklist

Changes follow CLAUDE.md conventions
No unwrap() in production code — use expect("should ...")
Uses repository logging macros (not println!)
New code has tests
No secrets or credentials committed

ChristianPavilonis

Summary

The parser-aware seam paths are well covered, but the new RSC streaming implementation introduces CPU-scaling, false-positive matching, and chunk-dependent fallback behavior that should be corrected before merging.

[P3] Update the deprecation guidance to name an available replacement (crates/trusted-server-core/src/integrations/nextjs/html_post_process.rs:98)

The public functions are still re-exported for compatibility, but their documentation and compiler deprecation notes tell callers to use NextJsHtmlPostProcessor, which this PR deletes.

Please point callers to the enabled Next.js streaming integration, or state that these compatibility functions have no direct public replacement.

aram356

Summary

Parser-confirmed body seam and bounded Next.js streaming, verified by execution rather than inspection: with the Next.js processor registered a non-final chunk emits real output pre-EOF, and a false </body> literal inside RSC data now streams past while the auction is still pending. BodyCloseHoldBuffer and every auction-coordination scan for </body are gone, and all 19 CI checks pass.

Two findings block. The RSC_PUSH_CALL_PATTERN widening changes behaviour for scripts that are not Next.js RSC at all, and the T-chunk escape scanner panics on publisher-controlled input reachable through the new classification path. Neither comment carries a one-click suggestion: I applied the obvious regex narrowing in a scratch worktree and it broke html_processor_rewrites_rsc_stream_payload_with_chunked_input, so the fix is described in prose instead of proposed as bytes I could not stand behind.

Blocking

🔧 wrench

Widened RSC push pattern rewrites unrelated publisher scripts — see inline at crates/trusted-server-core/src/integrations/nextjs/shared.rs:21
Escape scanner panics on non-char-boundary indices — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc.rs:210

Non-blocking

🤔 thinking

Deferred seam token is minted by construction but claimed by convention — see inline at crates/trusted-server-core/src/publisher.rs:650
release_bypass requires every captured placeholder in the current chunk — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc_stream.rs:346

♻️ refactor

Parser-side classification passes usize::MAX instead of the configured bound — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc_placeholders.rs:114

⛏ nitpick

Parity test shares one auction counter across adapters — see inline at crates/trusted-server-integration-tests/tests/parity.rs:1112
Parity test uses Axum as its own oracle — see inline at crates/trusted-server-integration-tests/tests/parity.rs:1176

Cross-cutting / body-level findings

🏕 html_post_process.rs is dead surface, and the PR description overstates its removal. The description says the whole-document post-processor was removed; 231 lines went, but 674 remain. post_process_rsc_html and post_process_rsc_html_in_place have zero callers anywhere in crates/ outside this module's own tests (verified by workspace grep) — roughly 150 lines of production code including a second full lol_html re-parse in find_rsc_push_scripts, plus ~400 lines of test module, all compiled into the wasm build behind #[allow(deprecated)]. Their doc comments still point at NextJsHtmlPostProcessor, which this PR deletes, so the surviving documentation describes a type that no longer exists. There are no callers left to migrate, so CLAUDE.md's "migrate callers and delete legacy APIs" applies cleanly. Deleting the module and the mod.rs re-export would drop ~674 lines of wasm-compiled dead weight; at minimum the stale doc comments need fixing.
📝 The adapter regressions landed in a different crate than the plan specifies, leaving three CI gates blind to this path. The plan (docs/superpowers/plans/2026-09-07-850-parser-aware-body-hold-nextjs-streaming.md:839-891, Task 7 Steps 5 & 7) prescribes "one buffered route regression in each adapter test module", lists crates/trusted-server-adapter-{axum,cloudflare,spin}/tests/routes.rs under Modify, and gives per-adapter verification commands. None of those three files changed — git diff --name-only shows zero tests/routes.rs in this PR. The single fixture lives only in crates/trusted-server-integration-tests/tests/parity.rs. Consequence: cargo test-axum, cargo test-cloudflare, and cargo test-spin (CI gate 3) do not exercise the Next.js/auction path at all, and the new pub fn routes_with_settings_and_services added to all three adapters is dead code within its own crate — its only consumer is the parity suite. The plan's completion review does disclose the relocation, but the Step 5/7 recipe and the Modify list were never corrected, so the plan still claims coverage that does not exist. Either move a fixture into each adapter's own tests, or amend the plan and note explicitly that per-adapter gates do not cover this path.
📌 Escape-scanner panic is pre-existing on main, not introduced here — flagged because this PR widens its reach. I verified EscapeSequenceIter is byte-identical at the merge base and that the same payload reached it there through rewrite_rsc_scripts_combined_with_limit, so this is not a regression and is legitimately out of scope for a fix in this PR. It is called out because the new classify_rsc_group call site runs the scanner on borrowed publisher fragments inside a lol_html callback, which is a materially wider and earlier exposure than the old post-processor path. A follow-up issue is the right home if you would rather not grow this PR.

CI Status

integration tests: PASS
browser integration tests: PASS
integration tests (Fastly EC lifecycle): PASS
CodeQL: PASS
cargo test: PASS (required)
cargo test (ts CLI, native): PASS
vitest: PASS
format-typescript: PASS (required)
Analyze (javascript-typescript): PASS
cargo test (axum native): PASS
Analyze (rust): 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 fmt: PASS (required)
prepare integration artifacts: PASS
Analyze (actions): PASS
format-docs: PASS (required)

Address review feedback on the parser-aware body hold. Require a qualified `self.`/`window.` receiver again. The widened pattern matched `myAnalytics.__next_f.push([1,"url"])` in unrelated publisher scripts and rewrote their string literals. The bare form was load-bearing because the receiver can stream away before `__next_f` is recognised, so the receiver is now retained in a bounded context on the document state and verified out of band; an anchored pattern is used only once that context proves it. Reject a match whose preceding byte continues a member expression. Keep the escape scanner on character boundaries. `\x` advanced four bytes without checking, so publisher input such as `1:T9,\x4é` panicked the HTML pipeline and returned 500. Only the boundary is validated, not the hex digits, so the unescaped byte count that drives T-chunk length recomputation is unchanged for input that previously scanned. Classify a group incrementally. Rebuilding and rescanning the whole group per payload cost 2.4s for an 8.96MiB T-chunk over 256 payloads; feeding each payload once costs 19ms, and the cost no longer scales with segment count. Chunk content consumption and non-chunk segment inspection both resume where they stopped, and EOF reclassifies with nothing held back. Scope the captured-payload limits. `max_combined_payload_bytes` no longer bounds the parser queue in aggregate, so two independent payloads sharing a source chunk stop bypassing each other; the queue is bounded by the parser's own script-buffer budget and a payload count instead. Return the deferred inline seam token from `PublisherBodyProcessor::new` so a caller without a seam controller discards it visibly, and log when one is dropped, since a debug assertion is compiled out of the wasm builds that ship. Delete `html_post_process.rs`. It had no callers outside its own tests and documented a type this work removes. Share the Next.js auction fixture through `test-utils` so each adapter carries its own buffered regression and the parity suite stops duplicating it. Assert on the recomputed Flight `T` length rather than a substring the split fixture can never contain.

ChristianPavilonis

Summary

Reviewed 481b24f7de7d7e6e3bb55d7fe59297383a2f95e1 against 6cae7f5da8911c746cf873581885f90c3820dd96. The parser-confirmed auction seam and bounded Next.js RSC streaming paths are covered by focused WASM tests across supported encodings, and all reported CI checks pass. No actionable issues found.

aram356

Summary

Second review pass, against head 481b24f7d. Every finding from the previous pass is addressed, and I re-verified the two blocking ones by running the original repros rather than reading the diff: all four regex false positives now return None with every legitimate receiver form still matching, and all three previously-panicking escape inputs now scan cleanly. The seam-token constructor change is a better fix than the one proposed. html_post_process.rs is fully deleted, each adapter now carries its own buffered regression that genuinely runs under its own CI alias, and the plan doc records each deviation instead of silently dropping it. I also reproduced the classifier performance claim independently: 8.75 MiB across 256 segments classifies in 26ms.

The hardening commit introduced two new defects, both in the fixes themselves. Neither corrupts a response — both degrade to the hydration-safe fallback — but the first one undoes the streaming behaviour this PR exists to deliver, so it should not merge as-is.

One of the inline comments carries a one-click suggestion; it passed the full CI-equivalent gate in isolation (cargo fmt, clippy-fastly, clippy-axum, test-fastly, test-axum, test-cloudflare, test-spin, and the cross-adapter parity suite), with a byte-exact drift check confirming the posted bytes are the bytes that were tested. The other blocking finding is prose-only on purpose: its fix interacts with a length-counting invariant described in that comment, and proposing bytes I had not proven safe would be worse than describing the constraint.

Blocking

🔧 wrench

Escape hold-back defers every realistic Flight payload, defeating streaming — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc.rs:302
Over-limit trimmed claim always forces document-wide RSC bypass — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc_placeholders.rs:132 (carries a suggestion)

Non-blocking

🤔 thinking

Incremental classifier can latch Invalid where a full rescan reports NeedMore — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc_stream.rs:1224

⛏ nitpick

Split-invariance test compares the classifier against itself — see inline at crates/trusted-server-core/src/integrations/nextjs/rsc_stream.rs:1224
Broken intra-doc link to a symbol that does not exist — see inline at crates/trusted-server-core/src/integrations/nextjs/shared.rs:18
New public context fields carry no doc comments — see inline at crates/trusted-server-core/src/integrations/registry.rs:557

Cross-cutting / body-level findings

🌱 Lazy Fastly disconnect still records no abandonment telemetry, and this PR is not the cause. A client disconnect mid-stream on the lazy path leaves DispatchedAuctionGuard::drop logging a log::warn! with no terminal abandoned row, so the SSP fan-out and its quota consumption vanish from telemetry. The write-sink driver now handles this correctly via the new abandon_hold_auction(..., "stream_write_error") wrapper. I want to be precise about scope, because the asymmetry invites the wrong conclusion: origin/main carries the byte-identical warn-only Drop impl, and stream_write_error does not appear anywhere on main. So this PR added abandonment telemetry to one driver and left the other exactly as it found it. That is a net improvement, not a regression, and it should not gate this PR. Worth a follow-up issue, with the note that a generator drop is not an Err, so the write-sink's is_err() branch will not port across directly: it needs a guard-side terminal emit or an explicit drop hook.
📝 Worst-case concurrent RSC memory now compounds across four buffers. The captured-payload queue bound moved from max_combined_payload_bytes (10 MiB default) to ctx.max_buffered_script_bytes (publisher.max_buffered_body_bytes, 16 MiB default), and RscGroupClassifier now retains combined for the lifetime of a group, which is a second copy of bytes already held in self.group (the previous classify_rsc_group built and dropped that string per call). Worst case, concurrently live: roughly 16 MiB queue + 10 MiB held_output + 10 MiB combined + 10 MiB group. I cannot demonstrate an OOM, and each individual bound is defensible, but the two changes compound and the total is worth confirming against the Fastly Compute heap budget rather than discovering at peak traffic.
📌 migration_guards.rs's allowlist is hand-maintained and silently incomplete. Out of scope here and not caused by this PR, which correctly adds its one new core file (rsc_stream.rs) and removes the deleted one. Flagging because the guard's value depends on the list: roughly two dozen core modules are absent from it (auction/plan.rs, config.rs, platform/template_cache.rs, price_bucket.rs, among others), so a future file added without a list entry is unguarded with no failing test to say so. A directory-walk-based guard would remove the maintenance burden. None of the missing modules were touched by this PR.

CI Status

All 20 checks reported by gh pr checks are PASS, including the four required gates. I also re-ran cargo fmt --all -- --check and cargo clippy-fastly against a clean export of this head, both clean, and cargo test -p trusted-server-core --lib reports 2702 passed / 0 failed.

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

prk-Jr added 14 commits September 7, 2026 19:08

Specify parser-aware body hold and Next.js streaming

739ac6c

Clarify bounded Next.js streaming states

02f56b8

Complete Next.js overflow transitions

a0b5481

Define RSC bypass flush transition

545a98c

Plan parser-aware body and Next.js streaming

f33a56f

Address issue 850 plan review

32de022

Make adapter parity plan executable

123ef68

Classify bounded Next.js RSC groups

b177c27

Isolate bounded Next.js script capture

304504c

Add per-document HTML stream processors

e5389fb

Stream bounded Next.js RSC groups

f297e65

Resolve auctions at parser-confirmed body seams

93cfb3f

Document bounded Next.js streaming

6d20f24

Resolve streaming fallback review findings

5bfce98

prk-Jr self-assigned this Sep 7, 2026

prk-Jr and others added 4 commits September 9, 2026 20:43

Complete parser-aware streaming and resolve review findings

6f5d329

Merge main and preserve parser-aware streaming

5f1d847

Restore fragmented Next.js scripts when streaming falls back

d561385

Merge branch 'main' into fix/850-parser-aware-body-hold-nextjs-streaming

5bf9ed6

prk-Jr marked this pull request as ready for review September 10, 2026 12:05

prk-Jr requested review from ChristianPavilonis and aram356 and removed request for aram356 September 10, 2026 12:05

ChristianPavilonis requested changes Sep 10, 2026

View reviewed changes

Comment thread crates/trusted-server-core/src/integrations/nextjs/rsc_stream.rs Outdated

Comment thread crates/trusted-server-core/src/integrations/nextjs/shared.rs Outdated

Comment thread crates/trusted-server-core/src/integrations/nextjs/rsc_placeholders.rs

aram356 requested changes Sep 10, 2026

View reviewed changes

prk-Jr and others added 2 commits September 11, 2026 17:32

Merge branch 'main' into fix/850-parser-aware-body-hold-nextjs-streaming

481b24f

prk-Jr requested review from ChristianPavilonis and aram356 September 16, 2026 13:17

aram356 requested a review from dhruv8sh September 17, 2026 15:16

ChristianPavilonis approved these changes Sep 17, 2026

View reviewed changes

aram356 requested changes Sep 21, 2026

View reviewed changes

Restore safe incremental RSC streaming

66e17c4

prk-Jr requested a review from aram356 September 21, 2026 04:32

dhruv8sh requested changes Sep 21, 2026

View reviewed changes

Comment thread crates/trusted-server-core/src/integrations/nextjs/rsc_placeholders.rs

prk-Jr and others added 2 commits September 21, 2026 12:05

Merge branch 'main' into fix/850-parser-aware-body-hold-nextjs-streaming

3363ee4

Check every push in oversized RSC fragments

bd18e23

prk-Jr requested a review from dhruv8sh September 21, 2026 06:48

dhruv8sh approved these changes Sep 21, 2026

View reviewed changes

Merge branch 'main' into fix/850-parser-aware-body-hold-nextjs-streaming

c7822e9

Navigation Menu

Make body hold parser-aware and stream Next.js processing - #1135

Make body hold parser-aware and stream Next.js processing#1135
prk-Jr wants to merge 24 commits into
mainfrom
fix/850-parser-aware-body-hold-nextjs-streaming

prk-Jr commented Sep 7, 2026

Uh oh!

ChristianPavilonis left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aram356 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis left a comment

Uh oh!

aram356 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

prk-Jr commented Sep 7, 2026

Summary

Changes

Closes

Test plan

Checklist

Uh oh!

ChristianPavilonis left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

🤔 thinking

♻️ refactor

⛏ nitpick

Cross-cutting / body-level findings

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ChristianPavilonis left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

aram356 left a comment

Choose a reason for hiding this comment

Summary

Blocking

🔧 wrench

Non-blocking

🤔 thinking

⛏ nitpick

Cross-cutting / body-level findings

CI Status

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants