Skip to content

fix: thread Transport/Host through all remote code paths - #139

Merged
gavin-jeong merged 1 commit into
masterfrom
fix/remote-ssh-transport-threading
Jul 25, 2026
Merged

fix: thread Transport/Host through all remote code paths#139
gavin-jeong merged 1 commit into
masterfrom
fix/remote-ssh-transport-threading

Conversation

@gavin-jeong

Copy link
Copy Markdown
Collaborator

What

The audit found that many remote code paths built a k8s-only Config from saved sessions, bypassing the Transport interface — so SSH sessions were deleted by the phase poller, could not re-attach after restart, could not exec/pull/snapshot/stop, and were cleaned up on transient network failures.

Fixes (10 changes, all Transport/Host threading)

  1. pollRemotePhasesCmd: builds Config with Transport/Host, uses Transport.Status instead of PodPhase. "unreachable"/"unknown" keep the session (don't delete); only "NotFound" deletes.
  2. cleanupStaleRemoteSessions: same — keeps unreachable/unknown, only drops ended/failed/stopped/notfound.
  3. resolveRemoteConfig: threads Transport/Host/LocalDir from saved session through mergeRemoteConfig + Defaults.
  4. attachToRemoteSession (saved): same threading + Defaults so SSH re-attach works and k8s gets RemoteUser/WorkDir filled.
  5. stopRemoteSessionInternal + saved-session stop: uses FetchWorkdirToDirWithTransport (new) instead of FetchWorkdirToDir; pull guard on LocalDir (not Context) so SSH can pull; Release via Transport instead of DeletePod.
  6. executeCmdRemoteExec: uses Transport.Exec instead of ExecInPod.
  7. executeCmdRemotePhase: uses Transport.Status instead of PodPhase.
  8. deleteSession (app.go): uses Transport.Release instead of DeletePod.
  9. fetchRemoteWorkdir: refactored to use Transport.Exec (new fetchRemoteWorkdirWithTransport); FetchWorkdirToDirWithTransport added for callers with a live Transport.
  10. handleRemotePhase: SSH status label uses "host [status]" format; duplicate UpdateSavedSessionStatus removed.

Impact

  • SSH remote sessions survive the 30s phase poller and ccx restarts.
  • :remote:exec, :remote:pull, :remote:stop, :remote:phase, and delete all work for SSH.
  • "unreachable" (VPN off, laptop offline) no longer deletes the saved session.
  • Re-attach after restart uses the correct transport and fills RemoteUser/WorkDir.

go build / go vet clean; go test ./... green. Follow-up to #125-#138.

The audit found that many remote code paths built a k8s-only Config from
saved sessions, bypassing the Transport interface — so SSH sessions
were deleted by the phase poller, could not re-attach after restart,
could not exec/pull/snapshot/stop, and were cleaned up on transient
network failures.

Fixes (all in one pass):

1. pollRemotePhasesCmd: builds Config with Transport/Host, uses
   Transport.Status instead of PodPhase. "unreachable" and "unknown"
   keep the session (don't delete); only "NotFound" deletes.

2. cleanupStaleRemoteSessions: same — keeps unreachable/unknown, only
   drops ended/failed/stopped/notfound.

3. resolveRemoteConfig: threads Transport/Host/LocalDir from saved
   session through mergeRemoteConfig + Defaults.

4. attachToRemoteSession (saved): same threading + Defaults so SSH
   re-attach works and k8s gets RemoteUser/WorkDir filled.

5. stopRemoteSessionInternal + saved-session stop: uses
   FetchWorkdirToDirWithTransport (new) instead of FetchWorkdirToDir;
   pull guard on LocalDir (not Context) so SSH can pull; Release via
   Transport instead of DeletePod.

6. executeCmdRemoteExec: uses Transport.Exec instead of ExecInPod.

7. executeCmdRemotePhase: uses Transport.Status instead of PodPhase.

8. deleteSession (app.go): uses Transport.Release instead of DeletePod.

9. fetchRemoteWorkdir: refactored to use Transport.Exec (new
   fetchRemoteWorkdirWithTransport); FetchWorkdirToDirWithTransport
   added for callers with a live Transport.

10. handleRemotePhase: SSH status label uses "host [status]" format;
    duplicate UpdateSavedSessionStatus call removed.

@jinsekim jinsekim left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@jinsekim jinsekim added the auto-review/approved Auto-approved by the Slack auto-reviewer bot label Jul 25, 2026
@gavin-jeong
gavin-jeong merged commit fd59a9d into master Jul 25, 2026
3 checks passed
@gavin-jeong
gavin-jeong deleted the fix/remote-ssh-transport-threading branch July 25, 2026 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review/approved Auto-approved by the Slack auto-reviewer bot

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants