Skip to content

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

Merged
sahil87 merged 2 commits into
mainfrom
260813-2s4u-copilot-review-followups
Aug 13, 2026
Merged

fix: code-server respawn review follow-ups (from #583 Copilot review)#584
sahil87 merged 2 commits into
mainfrom
260813-2s4u-copilot-review-followups

Conversation

@sahil-noon

Copy link
Copy Markdown
Collaborator

Summary

PR #583 was merged before its Copilot review fixes were pushed, stranding the fix commit on the merged branch. This re-lands those three review fixes on top of main.

Changes

  • respawnOutcome gains an explicit respawnFailed zero value — error paths no longer return a zero value that reads as "done" (Copilot comment on code_server.go:216)
  • migrateForeignCodeServerSession no longer treats an unresolvable managed binary as "ours" — the no-anchor case now prints a note instead of silently skipping the migration check (code_server.go:277)
  • daemon.CodeServerSessionCommand doc comment states the daemon-liveness gate obligation — callers must gate on daemon.IsRunning before calling (tmux probes on a dead socket birth a server) (codeserver.go:366)

@sahil-noon
sahil-noon marked this pull request as ready for review August 13, 2026 11:34
@sahil87
sahil87 merged commit d6122f9 into main Aug 13, 2026
5 checks passed
@sahil87
sahil87 deleted the 260813-2s4u-copilot-review-followups branch August 13, 2026 11:35
@sahil-noon

Copy link
Copy Markdown
Collaborator Author

Added two production fixes observed on a real install attempt:

  1. Extraction refused every real tarballnode_modules/.bin's up-and-back-down relative symlinks (.bin/esvalidate -> ../esprima/bin/esvalidate.js) hit the ../-prefix ban from the feat: Own the code-server Install #582 hardening. The lexical check is now containment against the link's resolved parent; the post-creation resolution check stays as the escape backstop. Regression test added with the exact reported shape, plus an env-gated manual test verified against the real 227MB v4.132.0 tarball.
  2. Release resolve bound 10s → 30s — back-to-back context deadline exceeded on api.github.com observed before a third attempt succeeded.

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.

2 participants