Skip to content

fix(server): quitting no longer restarts the relay client mid-shutdown - #33628

Merged
juliusmarminge merged 3 commits into
mainfrom
fix/issue-33623
Nov 10, 2026
Merged

juliusmarminge merged 3 commits into
mainfrom
fix/issue-33623

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Problem

On quit, the desktop app sends SIGTERM to the backend's whole process group. The relay client (cloudflared, spawned detached: false) is in that group, so it exits cleanly on its own. The supervisor sees the exit while the desired config is still set, because only the runtime's own finalizer clears it, and that finalizer runs after slower shutdown steps such as stopping an OpenCode server. So it logs "Relay client exited; restarting" and spawns a replacement. The finalizer then has to stop that replacement mid-registration, which adds about 2.3s to the backend stop. A terminal Ctrl+C (SIGINT to the foreground group) does the same.

Reproduced on main with the real CloudManagedEndpointRuntime, a stub cloudflared in the same process group, and a 1s shutdown step ahead of the runtime's finalizer. Signalling the whole group gave the restart and stops of 3351ms (SIGTERM) and 3353ms (SIGINT). Without the slow step there was no restart and the stop took 91ms.

Change

The runtime records when the server receives SIGINT or SIGTERM. It uses the same scoped listener pattern ProviderSessionManager already uses for group signals, and removes the listeners when the layer closes. Once a signal has arrived, superviseConnector still stops the exited connector but returns before restarting it or requesting recovery. Normal crash restarts are unchanged.

Scope and approval

Fixes #33623

Verification

  • New tests does not restart a connector that exits after the server receives SIGINT / ... SIGTERM in apps/server/src/cloud/ManagedEndpointRuntime.test.ts. On main both fail with expected [ 400, 401 ] to deeply equal [ 400 ], because a replacement is spawned. With the fix, all 25 tests in the file pass.
  • vp run check:changed on the head: format, lint, knip, typecheck and tests pass.

Evidence

Server-only fix, no rendered surface, so the evidence is timestamped runtime transcripts rather than screenshots. Before = 53bcb9af7a (main), after = e039e17308. Same driver, stub and runner on both sides; each run uses a fresh test base directory and synthetic config.

Environment: Linux 6.1.102 x86_64, Node v26.13.0. The real CloudManagedEndpointRuntime and Node spawner under NodeRuntime.runMain, with a stub cloudflared in the server's process group, signalled as a group the way the desktop app does. Not a real Cloudflare tunnel.

Row Before (main) After (fix) Result
A1 SIGTERM to group, 1s shutdown step ahead of the relay finalizer transcript, stop 3372ms transcript, stop 1073ms before restarts the relay client and kills the replacement mid-registration; after does not restart
A2 SIGINT to group, same transcript, 3349ms transcript, 1042ms same as A1
A3 SIGTERM, no slow step (control) transcript, 68ms transcript, 72ms no restart on either side
B1 connector crashes (exit 1) with no signal, then shutdown transcript, 63ms transcript, 66ms crash restart and recovery request unchanged
B2 connector exits while the reconcile lock is held; signal before release transcript, 3360ms transcript, 1041ms before restarts; after does not
B3 signal during the 1000ms crash-loop backoff transcript, 3955ms transcript, 1641ms before spawns a third connector after the delay; after does not
C2/C3 layer closed without a signal; listener counts; rebuild transcript transcript after: SIGINT/SIGTERM listeners 1→2→1, host listeners preserved; a fresh layer still restarts a crashed connector

What else changed: while live, the runtime adds one SIGINT and one SIGTERM listener, removed on close. Control rows (A3, B1, C2/C3) keep the same spawn and recovery counts, with single-run timing differences of a few ms. Nothing else differs between the transcripts beyond PIDs and timestamps.

Not verified: macOS desktop with a real cloudflared and a linked T3 Connect account.

Evidence index · fixture matrix · scripts (not committed): driver.ts, cloudflared-stub.sh, cloudflared-stub.mjs, runner.py, run.sh · raw recordings

AI-assisted; verified with the tests and tools listed above.

juliusmarminge and others added 2 commits November 10, 2026 00:33
…33623)

These tests reproduce #33623: when the server receives
SIGINT or SIGTERM and the relay client in the same process group exits,
the supervisor restarts it because the desired config is only cleared by
the runtime's finalizer later in shutdown.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SIGINT and SIGTERM reach the whole process group, so the relay client exits
before the runtime's finalizer clears the desired config. The supervisor then
started a replacement that the finalizer stopped mid-registration, which
delayed backend stop by about 2.3s.

The runtime now records the shutdown signal, using the same listener pattern
as ProviderSessionManager, and the supervisor skips the restart and the
recovery request once the signal has arrived.

Fixes #33623

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. label Nov 10, 2026
@github-actions github-actions Bot added the size:S 10-29 changed lines (additions + deletions). label Nov 10, 2026
@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: 9a0d027 · PR result: e039e17 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@macroscopeapp

macroscopeapp Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 96aec6c

Macroscope's review found this PR approvable — Prevents the relay client from restarting after the server receives a shutdown signal. The change is confined to shutdown handling and retains existing cleanup and normal recovery behavior.

No code changes detected at e039e17. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The runtime now records SIGINT and SIGTERM and removes their listeners during finalization. If either signal was received, the connector supervisor skips recovery and restart. Tests verify this behavior for both signals.

Changes: Connector shutdown handling

Layer / File(s) Summary
Track shutdown signals
apps/server/src/cloud/ManagedEndpointRuntime.ts
The runtime registers SIGINT and SIGTERM listeners that set a shutdown flag. It removes both listeners during finalization.
Suppress connector restarts during shutdown
apps/server/src/cloud/ManagedEndpointRuntime.ts, apps/server/src/cloud/ManagedEndpointRuntime.test.ts
The supervisor skips recovery and restart when a shutdown signal was received. Tests invoke each signal listener and verify that the connector is not restarted.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

  • pingdotgg/t3code#9386: Changes connector crash supervision to queue managed-tunnel recovery requests; this PR adds a shutdown guard to the same post-exit supervisor path.
  • pingdotgg/t3code#24946: Requests connector restarts by killing the active process and relying on the supervisor to start a replacement; this PR suppresses replacements after shutdown signals.

Suggested labels: bugfix, size:S

Suggested reviewers: only21mil, macroscopeapp

Merge Risk: Minimal · up to 96aec

The relay shutdown change is mergeable after normal checks; the added signal listeners do not block server shutdown.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly summarizes the main change: the relay client no longer restarts during shutdown.
Linked Issues check Passed Issue #33623 requires the relay client to remain stopped during server shutdown. ManagedEndpointRuntime.make records SIGINT and SIGTERM in scoped listeners and removes them when the layer closes. Af…
Out of Scope Changes check Passed The changes are limited to shutdown handling in ManagedEndpointRuntime.ts and tests for that behavior in ManagedEndpointRuntime.test.ts. Both changes support issue #33623. No unrelated changes are…
Description check Passed The description covers the problem, change, linked issue, and focused verification results. It also states what was not verified. The required sections are present and mostly complete.
Full details: Linked Issues check

Explanation

Issue #33623 requires the relay client to remain stopped during server shutdown. ManagedEndpointRuntime.make records SIGINT and SIGTERM in scoped listeners and removes them when the layer closes. After a connector exits, the supervisor stops it and checks the shutdown flag before restart or recovery. Added tests cover both signals and verify that no replacement starts. The change preserves normal crash restarts when no shutdown signal was received.

Full details: Out of Scope Changes check

Explanation

The changes are limited to shutdown handling in ManagedEndpointRuntime.ts and tests for that behavior in ManagedEndpointRuntime.test.ts. Both changes support issue #33623. No unrelated changes are present in the reviewed diff.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between bbdc7dc and 96aec6c.

Files selected for processing: 2

  • apps/server/src/cloud/ManagedEndpointRuntime.test.ts
  • apps/server/src/cloud/ManagedEndpointRuntime.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed pr:in-progress A lead is working on this PR and removed pr:gate-failed A land gate failed labels Nov 10, 2026
@juliusmarminge juliusmarminge added pr:ready Every gate passed; waits for Julius's land label and removed pr:in-progress A lead is working on this PR labels Nov 10, 2026
@juliusmarminge
juliusmarminge merged commit 5636b56 into main Nov 10, 2026
46 of 47 checks passed
@juliusmarminge
juliusmarminge deleted the fix/issue-33623 branch November 10, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:ready Every gate passed; waits for Julius's land label size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Quitting restarts the T3 Connect relay client mid-shutdown, adding about 2.3s to the backend stop

1 participant