Repository navigation
fix(web): share H2 initialization and join client closure - #49
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Interacting initialization, authentication, and shutdown synchronization changes warrant final human review despite no confirmed defects.
Review effort: Balanced
Findings: None
What changed in this PR
Updates the web-cover H2 client to share cold initialization across callers and wait for owned setup and cleanup during shutdown.
Changes:
- Shares initialization results while keeping caller cancellation independent.
- Defers authentication until a live caller reserves a stream and handles invalid dial results.
- Adds lifecycle regression tests and documents the ownership guarantees.
| File | Description |
|---|---|
| internal/tunnel/web_h2_initialization_test.go | Tests cancellation and shutdown at initialization handoff. |
| internal/tunnel/web_h2_dial_sharing_test.go | Tests shared attempts, retries, cancellation, and authentication. |
| internal/tunnel/web_h2_dial_result_test.go | Tests caller preflight errors and dial-result ownership. |
| internal/tunnel/web_h2_close_join_test.go | Tests shutdown waiting for late dials and detached cleanup. |
| internal/tunnel/web_h2_client.go | Implements shared initialization and coordinated closure. |
| docs/WEB_COVER.md | Documents H2 initialization and shutdown guarantees. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c47c00b0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
Validation
c4ce94ddemonstrated cancellation/failed-connection ownership, non-joined Close, and non-shared initialization gaps before the implementation.4c47c00: queued callers received replacement error objects and two physical dial calls instead of one; the later real verifying TLS/H2 two-flow full/short-auth echo controls still succeeded. The follow-up captures the selection cohort before queue admission and retains its immutable result.make checkandmake raceon the repaired source pass. Owned healthy controls exercise actual numeric-loopback verified TLS/H2, full/short authentication, physical reuse and complete TCP echo payloads.Scope
No dependency/native-protocol change, new release/tag, registry publication, SSH deployment or formal browser/PCAP comparison. These changes apply to source builds after v1.0.1, not the existing release binary. No browser equivalence, general connection-speed or recognition ranking claim. Close joins our owned workers, not every dependency goroutine; cancellation-ignoring custom callbacks must still return.