Skip to content

test: prove downstream H2 concurrent-stream multiplexing - #77

Open
seonghobae wants to merge 21 commits into
fix/downstream-tls-explicit-security-profile-v1from
test/downstream-h2-multiplexing-v1
Open

test: prove downstream H2 concurrent-stream multiplexing#77
seonghobae wants to merge 21 commits into
fix/downstream-tls-explicit-security-profile-v1from
test/downstream-h2-multiplexing-v1

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Refs #51. Ready-for-review writer-safe child of #76. Parent is exact 2c4433b99c539a43bdecc74c7d167b2446dd4fdc; current child exact is 0d583374607f32876fa17ef0501a8a548fb074f1. No force push or destructive rebase has been used.

This PR owns executable real-wire HTTP/2 multiplexing evidence only. One certificate-verified h2 downstream connection sends stream 1 (/) and stream 3 (/index.html) before either response is read. The H1 origin withholds response until a second distinct origin connection appears within 3 s, so a serialized gateway cannot manufacture GREEN; both streams must remain live, reach the expected paths, avoid RST_STREAM, return distinct bodies and terminate independently.

Fresh review of predecessor fc203de8... found a fixture-only TOCTOU: traffic/metrics reservation listeners were dropped long before config completion/process spawn, allowing unrelated port reuse to fail the H2 acceptance. Ordinary forward fb677251c4f255d1b0b78a94b53433f003cf13db repairs that by holding both TcpListener reservations through config construction and consuming/dropping them inside spawn_gateway() immediately before process launch. H2 frame/origin/concurrency/timing semantics did not change.

Parent #76 then moved from source/test repair 7a5f5e... to ADR-only exact 2c4433.... This child inherited the current ADR in ordinary forward 8bab2babe8d39786455ad1fee165cbd14cd430d7, then current 0d583374... was created as a normal two-parent merge with #76 2c4433...; both ref movements were fast-forward/non-force. Fresh parent→child comparison is ahead-only, merge base exactly 2c4433..., and effective child delta remains only tests/downstream_tls_h2_multiplexing_wire.rs.

Exact-current CI 34558985477, Supply Chain 34558985542, and PgErd bounded-origin capacity 34558985485 are all terminal GREEN. No inline review threads are open. Exact-current terminal technical COMMENT 5174930925 re-read the sole effective child path after execution and found no additional writer-safe source/test defect; it is technical evidence only, not independent APPROVED governance credit.

The branch-local multiplexing implementation/execution gate is therefore closed for independent review and this PR is Ready rather than Draft. This evidence does not close H2 stream cancellation/reset, GOAWAY/drain, decoded header/body admission, connection/stream flow control/backpressure, origin failure/recovery, forwarding trust, handshake/reuse performance, H3/QUIC, supplier #901 Cookie coalescing or release-qualified #936/#976-equivalent zero-length body disposition.

The active organization ruleset still requires one approving review, resolved review threads and central required workflows before default-branch integration. No bypass, protected merge, immutable release, canary/shadow, rollback, cutover or legacy-removal credit is claimed.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 668e8225-6df7-4499-8757-40142088d8bd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current writer review on 04b1329ae9eba5b048e18a29dfb5b178bc253b0c: verified the non-force parent restack and the subsequent fixture-only repair. The PR-visible delta remains one real-wire H2 multiplexing test. Its origin helper no longer assumes a complete HTTP/1 request arrives in one TcpStream::read; it reads until the header terminator under the existing 5 s deadline and fails closed above 64 KiB. This prevents TCP segmentation from manufacturing a false multiplexing failure without weakening the core oracle: streams 1 and 3 must still be issued on one verified h2 connection before either response, require two live H1 origin connections within 3 s, return distinct bodies, avoid RST_STREAM and terminate independently. No production Rust/Admin Config/TLS policy/routing/auth or external authority changed. I found no further writer-safe defect in this focused current delta. COMMENT only; exact CI/Supply Chain/capacity are nonterminal and this is not independent APPROVED or promotion credit.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current writer review on 32cd55d5a838f38db4dcd0e1bc6e7da02aed41db: the H2 multiplexing oracle remains unchanged from the repaired child exact 04b1329...; the only new tree movement is adoption of parent #76's complete-origin-header fixture repair by an ordinary two-parent merge. One verified h2 connection must still carry streams 1 and 3 concurrently, with a second live H1 origin connection required inside 3 s before either response is released, no RST_STREAM, distinct bodies and independent stream termination. No production code or H2 acceptance semantics changed and I found no further writer-safe defect in this current delta. COMMENT only; exact-current CI/Supply Chain/capacity are nonterminal and this is not independent APPROVED or promotion credit.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current writer review on c8a637e0c521edfbfabcc8ad13a394dca4eef9d0: parent a5dc77... is now in ancestry through ordinary merge 926c72902355d98034bfeab48bed54471ad23038; 6ffefe472ca5da31e4152b18f32c6fadda23c5ff restores the parent-fixed TLS profile fixture in the resolved tree, and current c8a637... applies the same absolute remaining-budget deadline to this PR's H2 multiplexing origin reader. The two-stream concurrency oracle, 3 s second-origin admission bound, frame assertions and production Rust are unchanged. COMMENT technical evidence only; new exact CI/Supply Chain/capacity are nonterminal and this is not independent APPROVED, merge, release or cutover credit.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current writer review on a4791b4db22f9748c77562a7519e8fabe2b9f1ab: ordinary two-parent merge adopts #76 exact a0bf55151255beea04fb305e721b5629b7c79b2e and therefore the parent TRACEABILITY repair, while preserving the H2 multiplexing acceptance tree at c8a637.... The single verified-H2/two-live-stream oracle, 3 s second-origin admission bound, absolute five-second origin-header budget, 64 KiB cap and production Rust remain unchanged. COMMENT technical evidence only; fresh exact CI/Supply Chain/capacity are queued and no predecessor GREEN or independent approval transfers.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current-head technical review after parent repair/restack. The previous #77 CI failure was inherited from #76's unformatted structural TLS contract; its H2 multiplexing file was not the failing path. This exact inherits the repaired TLS source and rustfmt-clean contract from current #76, preserves the H2 concurrency oracle unchanged, and was non-force restacked onto df7262d.... Exact parent→child comparison now contains only tests/downstream_tls_h2_multiplexing_wire.rs, which is the intended ownership boundary. Fresh exact-head gates must pass before Ready or merge; this COMMENT is not independent approval.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current-head follow-up after the #75/#76 rustfmt repair stack. This child inherits the formatter-clean TLS source, then non-force restacks onto #76 exact 273832d... via two-parent merge bee37f6.... Exact parent→child comparison is ahead-only and still contains only tests/downstream_tls_h2_multiplexing_wire.rs; no H2 oracle delta was introduced by the repair. Fresh exact gates must pass before Ready or merge; this COMMENT is not independent approval.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current restack review on 400cf5d67bd39e03d930472f8a8a31a73f9ca354. Prior exact bee37f61... failed CI at the inherited #76 formatting gate while Supply Chain and bounded-origin capacity were GREEN. The branch first inherited the exact Rust 1.98.0 import-layout repair as ordinary forward commit 3c4153cd806488d7ae3838938d5f4207ba9220b8, then non-force restacked onto current #76 bc34e6302efd53682b8a207ee73cac3600fe86b5 through a two-parent merge. Parent→child compare is ahead-only with merge base exactly bc34e630... and contains only tests/downstream_tls_h2_multiplexing_wire.rs, so the H2 multiplexing ownership boundary is preserved. Fresh exact CI/Supply Chain/capacity are queued. COMMENT technical evidence only; this is not independent APPROVED and Draft/merge/release/cutover credit remains withheld until exact-current gates terminate GREEN.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current restack review on 75ced2c6e51afa917db00be70faf94081a1e0e3f. This is an ordinary two-parent merge from prior child 400cf5d... and repaired #76 parent a1a1a5dc...; no force push or destructive rebase was used. Exact parent→child compare is ahead-only with merge base exactly a1a1a5dc... and contains only tests/downstream_tls_h2_multiplexing_wire.rs. The child therefore inherits the parent's coverage-boundary repair without changing its H2 concurrent-stream oracle: one verified H2 connection sends streams 1 and 3 before either response is read, while the H1 origin requires a second distinct origin connection before releasing responses. No H2 traffic threshold, TLS policy, supplier source, authority boundary or test semantics changed in this restack. Fresh exact CI/Supply Chain/capacity are required before Ready; this COMMENT is technical evidence only, not independent approval.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current restack review on fc203de8b56fff34d8070618a50198186fecd07f. Ordinary forward a3afce1a25839ad722715df71ec5157c81df13df inherited only #76's TLS coverage-evidence repair, then this exact head was created as a normal two-parent merge with current parent #76 7a5f5e04016e93a7fd515517051430d853dd3b45; the branch ref was fast-forwarded without force. Fresh parent→child comparison is ahead-only with merge base exactly 7a5f5e... and only tests/downstream_tls_h2_multiplexing_wire.rs as the effective child delta. The H2 concurrent-stream traffic oracle, timings and authority boundaries are unchanged. Fresh exact CI/Supply Chain/capacity are nonterminal, so predecessor evidence does not transfer and Draft remains appropriate. COMMENT technical evidence only, not independent APPROVED.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current fixture review on fb677251c4f255d1b0b78a94b53433f003cf13db. Fresh review found a valid acceptance-integrity defect in predecessor fc203de8...: reserve_distinct_loopback_addresses() returned only SocketAddrs, so both reservation listeners were dropped long before config completion and gateway spawn. That reopened a material test race where another process could claim either traffic or metrics port and make the H2 oracle fail for an unrelated bind reason. Current repair returns live TcpListener reservations, derives both addresses while they are held, writes the config, then consumes/drops both reservations inside spawn_gateway() immediately before launching the compiled gateway. This follows the repository's established held-listener fixture pattern and changes no H2 frame/origin/concurrency/timing semantics. Fresh parent→child comparison remains ahead-only with merge base exactly current #76 7a5f5e... and only tests/downstream_tls_h2_multiplexing_wire.rs as effective child delta. New exact CI 34558703085, Supply Chain 34558703059, and capacity 34558703060 are pending/queued, so predecessor evidence does not transfer and Draft remains appropriate. COMMENT technical evidence only, not independent APPROVED.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current non-force restack review on 0d583374607f32876fa17ef0501a8a548fb074f1. Parent #76 moved only by ADR-evidence commit to 2c4433b99c539a43bdecc74c7d167b2446dd4fdc. Child first inherited that exact ADR as ordinary forward 8bab2babe8d39786455ad1fee165cbd14cd430d7, then 0d583374... was created as a normal two-parent merge with current #76; both branch updates were fast-forward/non-force. Fresh compare is ahead-only, merge base exactly 2c4433..., and effective child delta remains only tests/downstream_tls_h2_multiplexing_wire.rs. The listener-reservation TOCTOU repair from fb677251... remains intact; H2 frames, concurrent-origin oracle and timing thresholds are unchanged. Fresh exact CI 34558985477, Supply Chain 34558985542, and capacity 34558985485 are nonterminal, so no predecessor evidence transfers and Draft remains appropriate. COMMENT technical evidence only, not independent APPROVED.

@seonghobae seonghobae left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact-current terminal review on 0d583374607f32876fa17ef0501a8a548fb074f1. Fresh execution is terminal GREEN across CI 34558985477, Supply Chain 34558985542, and PgErd bounded-origin capacity 34558985485. Re-read of the sole effective child path confirms the fixture still proves two live streams on one verified TLS/H2 connection before either origin response, with distinct H1 origin connections, no stream reset, distinct bodies, independent END_STREAM, a 3 s serialization bound, bounded complete-header reads, and held listener reservations through config construction to remove unrelated bind-race REDs. No unresolved review threads are present and no additional writer-safe source/test defect was found on this exact head. COMMENT technical evidence only; this is not independent APPROVED governance and does not authorize protected merge, release or cutover.

Copy link
Copy Markdown
Contributor Author

@coderabbitai approve

Current exact 0d583374607f32876fa17ef0501a8a548fb074f1 is terminal GREEN across CI/Supply Chain/capacity with no open inline review threads and only the intended H2 multiplexing acceptance delta over #76. Please review/approve the exact current head only; do not transfer any predecessor review.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Local commit identity and ancestry =="
git rev-parse HEAD
git show -s --format='commit=%H%nparents=%P%nsubject=%s' HEAD
git merge-base HEAD 2c4433b99c539a43bdecc74c7d167b2446dd4fdc
git diff --name-status 2c4433b99c539a43bdecc74c7d167b2446dd4fdc...HEAD
git diff --check 2c4433b99c539a43bdecc74c7d167b2446dd4fdc...HEAD

echo "== Changed test file structure and content =="
ast-grep outline tests/downstream_tls_h2_multiplexing_wire.rs --items all
cat -n tests/downstream_tls_h2_multiplexing_wire.rs

echo "== PR state, head, reviews, and checks =="
gh pr view 77 --repo ContextualWisdomLab/pingora-gateway \
  --json number,state,isDraft,headRefOid,baseRefOid,mergeStateStatus,reviewDecision,reviews,comments,statusCheckRollup \
  --jq '{
    number, state, isDraft, headRefOid, baseRefOid, mergeStateStatus, reviewDecision,
    reviews: [.reviews[] | {author: .author.login, state, commit: .commit.oid}],
    comments: [.comments[] | {author: .author.login, body}],
    checks: [.statusCheckRollup[] | {name, status, conclusion}]
  }'

Length of output: 26879


🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="ContextualWisdomLab/pingora-gateway"
sha="0d583374607f32876fa17ef0501a8a548fb074f1"

echo "== Check suites and runs for the exact head =="
gh api "repos/${repo}/commits/${sha}/check-suites?per_page=100" --paginate \
  --jq '.check_suites[] | {
    app: .app.slug,
    status,
    conclusion,
    head_sha,
    workflow_name: .workflow_run.name,
    workflow_id: .workflow_run.id,
    url: .html_url
  }'

echo "== Unresolved review threads for PR 77 =="
gh api graphql -F owner="ContextualWisdomLab" -F name="pingora-gateway" -F number=77 -f query='
query($owner: String!, $name: String!, $number: Int!) {
  repository(owner: $owner, name: $name) {
    pullRequest(number: $number) {
      reviewThreads(first: 100) {
        nodes {
          isResolved
          isOutdated
          path
          line
          comments(first: 1) {
            nodes { author { login } body }
          }
        }
      }
    }
  }
}' --jq '
  .data.repository.pullRequest.reviewThreads.nodes
  | map(select(.isResolved == false))
'

Length of output: 3322


@seonghobae Approved for exact head 0d583374607f32876fa17ef0501a8a548fb074f1.

I reviewed only the delta from 2c4433b99c539a43bdecc74c7d167b2446dd4fdc. The delta contains the intended H2 multiplexing wire test. I found no source or test defect in this scope. There are no unresolved review threads.

The GitHub Actions suites for this head are successful. Some third-party check suites remain queued and can affect merge eligibility independently.

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant