Skip to content

fix: code-server respawn review follow-ups (from #583 Copilot review) #584

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sahil87 merged 2 commits into from
Aug 13, 2026
+26 −8
Merged

fix: code-server respawn review follow-ups (from #583 Copilot review) #584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0cae660
fix: address review feedback from @Copilot
sahil87 Aug 13, 2026
6705dce
Update review-pr status
sahil87 Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions app/backend/cmd/rk/code_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,12 @@ func runCodeServerInstall(cmd *cobra.Command, _ []string) error {
type respawnOutcome int

const (
// respawnFailed is the deliberate zero value: it accompanies a non-nil
// error and carries no meaning on its own, so a forgotten assignment can
// never read as a successful respawn.
respawnFailed respawnOutcome = iota
// respawnDone: the session was killed and respawned on the managed binary.
respawnDone respawnOutcome = iota
respawnDone
// respawnDaemonDown: the daemon is not running — nothing was touched
// (no kill, no tmux probe). The helper prints nothing; the caller owns
// the recovery line.
Expand All @@ -228,11 +232,11 @@ func respawnCodeServerSession(sink outputSink, version string) (respawnOutcome,
}
sink.Notef("Restarting the code-server session on v%s...\n", version)
if err := codeServerKillFn(); err != nil {
return respawnDone, err
return respawnFailed, err
}
outcome, err := codeServerStartFn()
if err != nil {
return respawnDone, fmt.Errorf("respawning code-server (the new version IS installed — start it with `rk code-server start`): %w", err)
return respawnFailed, fmt.Errorf("respawning code-server (the new version IS installed — start it with `rk code-server start`): %w", err)
}
if outcome == daemon.EnsureExternallyManaged {
// StartCodeServer legitimately declines to respawn when the port is
Expand Down Expand Up @@ -272,8 +276,16 @@ func migrateForeignCodeServerSession(sink outputSink, home, version string) erro
return nil
}
managed := codeserver.ManagedBinary(home)
if managed == "" || strings.Contains(startCmd, managed) {
return nil // ours (or nothing to compare against) — no respawn
if managed == "" {
// Should not happen right after a successful install (the flip just
// activated a verified-executable binary) — but "no anchor" is NOT
// "managed", so say why the migration is not running rather than
// silently classifying the session as ours.
sink.Notef("Managed code-server binary not resolvable after the install — skipping the session migration check.\n")
return nil
}
if strings.Contains(startCmd, managed) {
return nil // ours — no respawn
}
out, err := respawnCodeServerSession(sink, version)
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions app/backend/internal/daemon/codeserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,11 @@ var codeServerPaneCommand = func(ctx context.Context, target string) ([]byte, er
// state). Callers (the `rk code-server install` migration respawn) classify
// the session by whether this string contains the managed binary path.
//
// Callers MUST gate on daemon liveness (daemon.IsRunning) BEFORE calling:
// this function runs tmux probes (has-session, list-panes), and any tmux
// command on a dead socket silently births a server — the same hazard the
// RunJob/StartCodeServer gates exist to prevent.
//
// Returns (cmd, exists, err): an absent session is ("", false, nil); a session
// that exists but cannot be inspected is ("", true, err) — the caller must
// treat that as uncertain evidence and never kill on it. list-panes, NOT
Expand Down
1 change: 1 addition & 0 deletions fab/changes/260813-2s4u-respawn-aware-code-server-install/.history.jsonl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
{"event":"review","result":"passed","ts":"2026-08-13T11:22:18Z"}
{"action":"enter","driver":"fab-fff","event":"stage-transition","stage":"ship","ts":"2026-08-13T11:24:04Z"}
{"action":"enter","driver":"git-pr","event":"stage-transition","stage":"review-pr","ts":"2026-08-13T11:25:32Z"}
{"event":"review","result":"passed","ts":"2026-08-13T11:34:31Z"}
6 changes: 3 additions & 3 deletions fab/changes/260813-2s4u-respawn-aware-code-server-install/.status.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ progress:
review: done
hydrate: done
ship: done
review-pr: active
review-pr: done
plan:
generated: true
task_count: 4
Expand All @@ -34,7 +34,7 @@ stage_metrics:
review: {started_at: "2026-08-13T11:11:36Z", driver: fab-fff, iterations: 1, completed_at: "2026-08-13T11:22:18Z"}
hydrate: {started_at: "2026-08-13T11:22:18Z", driver: fab-fff, iterations: 1, completed_at: "2026-08-13T11:24:04Z"}
ship: {started_at: "2026-08-13T11:24:04Z", driver: fab-fff, iterations: 1, completed_at: "2026-08-13T11:25:32Z"}
review-pr: {started_at: "2026-08-13T11:25:32Z", driver: git-pr, iterations: 1}
review-pr: {started_at: "2026-08-13T11:25:32Z", driver: git-pr, iterations: 1, completed_at: "2026-08-13T11:34:31Z"}
prs:
- https://github.com/sahil87/run-kit/pull/583
change_type_source: explicit
Expand All @@ -53,4 +53,4 @@ true_impact:
computed_at: "2026-08-13T11:25:33Z"
computed_at_stage: ship
# true_impact: lazily created on first stage-finish that computes it (no placeholder here).
last_updated: 2026-08-13T11:25:33Z
last_updated: 2026-08-13T11:34:31Z
Loading