Skip to content

Maintain the Jam patch set on current upstream with a weekly replay - #2

Merged
bandzalkin merged 5 commits into
jam-rustls-transportfrom
jam/rustls-transport-rebase
Oct 7, 2026
Merged

bandzalkin merged 5 commits into
jam-rustls-transportfrom
jam/rustls-transport-rebase

Conversation

@bandzalkin

Copy link
Copy Markdown
Collaborator

What this sets up

The Jam patch set as an org-owned maintenance branch with a weekly upstream replay. This is the same arrangement thenvoi/codex-sdk-rs uses, and it replaces the pin on darvell-thenvoi/copilot-sdk.

  • main stays an upstream mirror (it is at cf245cdf, the same as github/copilot-sdk main).
  • jam-rustls-transport (new, created at cf245cdf) becomes the maintenance branch: upstream main plus the commits in this PR, kept linear.
  • jam-rustls-transport-latest is created by the workflow on its first run. tjam's fork-freshness watcher compares it with tjam's Cargo.lock.

Commits

  1. rust: make the request-handler TLS backend a feature (default rustls). A backport of the open upstream PR rust: make the request-handler TLS backend a feature (default rustls) github/copilot-sdk#2811, unchanged. It drops out at the next manual rebase once that PR merges.
  2. rust: download the build-time CLI over rustls instead of native-tls. Upstream rust: use native-tls for the build-time CLI download github/copilot-sdk#1964 moved the build script to native-tls. Build dependencies are part of every consumer's resolved graph, so native-tls came back for Jam even with default-features = false. This commit uses ureq's rustls provider with the platform verifier and makes the matching dev-dependency change.
  3. Expose transport closure and reap disconnected Copilot clients. A port of Expose transport closure and reap disconnected clients #1 onto current upstream:
    • It uses upstream's existing connection-closed token rather than a second signal.
    • It adds Client::is_disconnected() and wait_for_disconnect().
    • A failed write closes the connection and cancels pending requests.
    • Requests sent after closure fail instead of hanging.
    • stop() skips impossible remote cleanup after confirmed loss and still clears routing and reaps the child.
  4. ci: keep the Jam patch set current with upstream. Adds .github/workflows/jam-rebase-rustls-transport.yml. It runs Mondays at 07:13 UTC and on demand. It replays the fork-only commits onto upstream main, then runs fmt, check --all-targets, and the lib tests. It also checks that native-tls/openssl-sys stay out of the --no-default-features --features rustls normal and build graph. It pushes jam-rustls-transport-latest only when the result changes.
  5. docs: record the Jam fork patch contract. Adds FORK-CHANGES.md. It covers each patch, what was dropped, recovery, and the tjam upgrade steps.

Two parts of the old fork are dropped:

  • The spawned userInput.request dispatch, because upstream now dispatches every inbound request on its own task.
  • The crate version stamp, because tjam pins by revision.

rust/Cargo.lock is deliberately left out of the patches, so upstream changes to the lockfile can't make the weekly replay conflict.

Validation (local, macOS arm64, toolchain 1.94.0)

  • I replayed the five commits onto upstream/main (cf245cdf) in a fresh clone, as the workflow does. The resulting tree is identical to this branch.
  • I ran the workflow's validation commands as written in the workflow, from rust/:
    • cargo fmt --all --check passed.
    • cargo check --all-targets --features test-support passed.
    • cargo test --lib --features test-support: 356 passed.
    • The dependency-graph check found no native-tls/openssl-sys. The same check run with --features native-tls flags it, so it can detect a regression.
  • cargo clippy --all-targets --features test-support -- -D warnings is clean.
  • The build_download and build_acquisition tests pass (15 + 4).
  • A consumer crate with default-features = false, features = ["rustls"] has no native-tls or openssl-sys for the macOS, Linux (x86_64-unknown-linux-gnu), and Windows (x86_64-pc-windows-msvc) targets. Before commit 2, native-tls came in through the build-script ureq.
  • There are 9 transport tests. Taking out each piece of the fix makes the matching test fail:
    • without the pending-request clear on write failure, the write-failure test fails;
    • without the check that drops writes after closure, the request-after-closure test fails;
    • without the transport-failure tolerance and router.clear() in stop(), the two stop tests fail.

Not yet verified: the workflow's first run on Linux CI. Its validation step runs before any push, so a red run publishes nothing.

Merge and setup

  1. Merge with Rebase and merge, not a merge commit or squash. The replay cherry-picks every fork-only commit, and it can't cherry-pick a merge commit.
  2. Settings → General → Default branch → jam-rustls-transport. Scheduled workflows run only from the default branch.
  3. Actions → jam-rebase-rustls-transport → Run workflow once. This creates jam-rustls-transport-latest.
  4. Merge Expose transport closure and reap disconnected clients #1 into jam/rustls-and-dispatch-v1.0.6-preview.1 with Create a merge commit. That keeps 01e70715d (tjam's current pin) on an org branch.

Opening this PR may also trigger upstream's own pull_request workflows in this fork. Some of them expect upstream secrets.

achicu and others added 5 commits October 7, 2026 10:07
Cargo unifies features across the dependency graph, so the hard-coded
reqwest/native-tls feature flipped reqwest's TlsBackend::default() to
native-tls for every reqwest client in a consumer's whole binary, not just
the SDK's request-handler transport. Introduce rustls (default, aws-lc-rs
via reqwest's rustls feature) and native-tls (opt-in) features instead.

Fixes github#1805

Backport of github#2811 (58a4cae) for Jam until it merges upstream.
Upstream github#1964 moved the build-script CLI download from ureq's rustls
stack to native-tls. Build dependencies are part of every consumer's
resolved graph, so that change put native-tls (and OpenSSL on Linux)
back into consumers that ban it, even with `default-features = false`
and no bundled CLI. Use ureq's rustls provider with the platform
verifier for the same OS trust store, and drop the native-tls probe.

rust/Cargo.lock is intentionally left to the next resolve so the weekly
replay does not conflict on upstream lockfile churn.
Copilot's native child can exit while its Node launcher remains alive.
Expose the SDK-owned transport lifecycle so embedders can observe it and
tear down cleanly:

- Client::is_disconnected() reports observed EOF, read failure, write
  failure, or explicit transport closure.
- Client::wait_for_disconnect() serves current and late waiters without
  polling.
- A failed write closes the connection and cancels pending requests;
  requests issued after closure fail instead of waiting forever.
- Client::stop() skips impossible remote cleanup after confirmed
  transport loss, clears local session routing, and still reaps the
  owned child. Healthy RPC failures remain reported.

Port of #1 (01e7071) onto current upstream main,
reusing upstream's connection-closed token as the closure signal.
Weekly replay of the fork-only commits onto github/copilot-sdk main into jam-rustls-transport-latest, validated before push. thenvoi/tjam's fork-freshness watcher turns a red run or a new staged revision into a tjam issue.
Documents each fork-only change, what was dropped since the v1.0.6-preview.1 fork, the weekly replay, manual conflict recovery, and the tjam upgrade procedure.
@bandzalkin
bandzalkin merged commit fd8c9a8 into jam-rustls-transport Oct 7, 2026
78 of 85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants