fix(ssh): restart managed server after runner update#4290
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
ApprovabilityVerdict: Needs human review This PR modifies process lifecycle management logic in the SSH tunnel code, changing when managed servers are stopped and restarted. While the diff is small and includes comprehensive tests, changes to process management behavior warrant human review due to potential edge cases. You can customize Macroscope's approvability policy. Learn more. |
What Changed
server-runtime.jsonwhen the SSH state is managed and the saved/runtime ports match.Why
The SSH launcher can save the npm/npx wrapper PID while the real T3 server continues under a different PID. When the runner changes, signaling the exited wrapper leaves the old server alive, and the launcher then reuses it as external. That keeps the remote environment on the previous T3 version.
The runtime descriptor is authoritative only when the existing managed marker and saved port correlate it with the launcher state, which lets the launcher restart its server without risking an unrelated process.
Addresses #3598.
Verification
pnpm --filter @t3tools/ssh test -- --runpnpm --filter @t3tools/ssh typecheckpnpm exec vp lint packages/ssh/src/tunnel.ts packages/ssh/src/tunnel.test.tspnpm exec vp fmt --check packages/ssh/src/tunnel.ts packages/ssh/src/tunnel.test.tsChecklist
Note
Fix managed SSH tunnel server to restart after runner update
tunnel.tspreviously scheduled the running managed server for termination when the runner updated, even if the server was still healthy on the correct port.$REMOTE_PIDfor termination rather than a fallback value.resolve_default_runtime_port.tunnel.test.tsverifies the full restart flow, including that the old server exits and the wrapper process survives.Macroscope summarized cae6e56.
Note
Medium Risk
Changes remote process kill/reuse logic on SSH hosts; incorrect PID handling could affect running servers, though scope is narrowed with port correlation and a regression test.
Overview
Fixes SSH remote launch so managed sessions track the real T3 server PID from
server-runtime.jsoninstead of a dead npm/npx wrapper, so a runner update actually restarts the old server instead of reusing it as external.The launch script now saves
SAVED_REMOTE_PORTbefore applying the default runtime port. When reuse is possible, it only adoptsDEFAULT_RUNTIME_PIDif managed state, the saved port, and the runtime port all match and that PID is alive—then it rewritespid. Otherwise it stopsREMOTE_PID(no fallback to the runtime PID) before demoting to external. After a fresh managed start, it resolves the runtime again and persists the server PID when the port matches.Adds a live shell test that simulates a live server, a stale wrapper PID in state, and a runner change; it asserts the old server exits and a new managed process replaces it.
Reviewed by Cursor Bugbot for commit cae6e56. Bugbot is set up for automated code reviews on this repo. Configure here.