fix: keep streaming terminal inside body

[Mohamed Mansour (mohamedmansour)]

Summary

• Legitimacy: legit bug
• Root cause: streaming terminal records were emitted only after traversal finished, so the parser had already written the raw </body></html> tail and the final boundary script landed after </html>.
• Fix: emit the terminal record at the structural body_end hook after body injections and before the raw closing tags, then coalesce it with the closing tail in the existing final flush.
• Docs: rewrote DESIGN.md into a compact high-level architecture/specification document and updated agent guidance so future changes keep it focused on durable contracts instead of implementation history.

Regression proof

• Added streaming_terminal_record_emits_before_body_close, which fails on the previous behavior because the terminal [0,4,0,{}] record appeared after </body>/</html>.

Performance and memory

• No new retained state, heap allocation, protocol field, or client JavaScript.
• Preserves the previous final flush count by coalescing the terminal record with the raw closing-tag tail, avoiding an extra tiny transport write.

Validation

• cargo test -p microsoft-webui-handler streaming_terminal_record_emits_before_body_close
• cargo test -p microsoft-webui-handler streaming_
• cargo test -p microsoft-webui-handler
• cargo xtask check with process-local TEMP/TMP under D:\webui-windows\target\pr-build-temp

[mddinbox]

Review Summary

• Target: 8d0fc32d...1eb3f6f9 (base...head)
• Coverage: complete diff (all 8 changed files), including running cargo test -p microsoft-webui-handler at head (36+ tests pass, including the new streaming_terminal_record_emits_before_body_close regression test)
• Challenge: independent
• Compatibility: the streaming code change is backward compatible (only reorders where the terminal record is written relative to </body>); the DESIGN.md rewrite is a spec/contract-documentation change, not a runtime compatibility break
• Verdict: request-changes

Findings

[medium] DESIGN.md rewrite silently drops durable contracts unrelated to this fix

• Location: DESIGN.md (whole-file rewrite, ~6848 -> ~333 lines)
• Confidence: high

The streaming-terminal-placement fix itself (crates/webui-handler/src/streaming/{mod,checkpoint,vm}.rs, lib.rs) is correct and well tested — verified by running the full microsoft-webui-handler suite at head, all green, plus manual trace confirming body_end now emits the terminal record exactly once before the raw </body></html> tail is written, and ContinuationVm::finalize only flushes/ends without re-emitting. However, this PR bundles a large, unrelated rewrite of DESIGN.md (and .github/copilot-instructions.md / two SKILL.md files) that redefines DESIGN.md from an exhaustive living specification to a condensed architecture doc. That rewrite drops concrete durable contracts that are still true of the implementation and were not carried anywhere else, e.g. the $webui reserved-state-channel rule "$webui is stripped from the client hydration payload for both full and projected state, so boundary HTML is never re-serialized into the DOM" is present in the base version but has no equivalent in the new version. Per this repo's own (also-being-edited-in-this-PR) rule that DESIGN.md is the living/mandatory specification, deleting such contracts without an exhaustive audit risks future implementers silently violating them, and this scope is unrelated to a PR titled "fix: keep streaming terminal inside body."

Fix: Split the DESIGN.md/instructions/skill restructuring out of this bug-fix PR into its own PR, with an explicit accounting of every durable contract that is condensed vs. dropped (e.g. restore the $webui "never hydrated" invariant, and audit the rest of the ~6500 removed lines for similar losses) so it can be reviewed on its own merits.

Findings verification

• Ran cargo test -p microsoft-webui-handler streaming_terminal and the full cargo test -p microsoft-webui-handler suite at PR head (1eb3f6f) in a scratch worktree: all pass.
• Confirmed via git show that the terminal record is emitted exactly once (in process_streaming_signal's body_end handling) and ContinuationVm::finalize no longer re-emits it, ruling out double-emission.
• Confirmed via git show diff of DESIGN.md at base vs. head that the $webui "Never hydrated" contract has no surviving equivalent in the new file.

[Mohamed Mansour (mohamedmansour)]

Addressed the changes-requested feedback by removing the DESIGN/guidance rewrite from the effective PR diff; the branch now carries only the streaming terminal placement fix plus its minimal spec update. I reran \cargo xtask check\ successfully before pushing.

[mddinbox]

Re-reviewed the current head 86f9eb8 against base 4f0f45b. The prior documentation-scope finding is resolved by the revert commits; the remaining streaming change emits the terminal record exactly once at body_end before , preserves the client record contract, and has complete passing CI including Windows. No new issues found.