[Jisung Chae (jisung-02)]

Fixes #454

Why

The review of #449 left three ways code-server teardown could still go wrong. A Stop in the spawn window, or a leader that died on its own, left the rest of the process group running. A stopped manager could still report ready. The fix the issue proposed for the spawn window (a context check after the process is published) does not close it: exec.CommandContext kills the leader alone on cancel, and once that kill is reaped the second Stop takes the "already reaped" path and signals nothing.

What changed

• A started code-server is one codeServerProcess handle. Its reaper waits for the leader to exit without collecting it (waitid with WNOWAIT on Linux, a kqueue NOTE_EXIT on macOS), SIGKILLs the process group, and only then reaps the leader. Stop's SIGTERM and its SIGKILL after the grace period take the same lock, so every signal to the group is sent while the leader is still unreaped. This covers every way the leader dies: a crash, the context's kill, or a SIGTERM its children outlive.
• Stop sets a final stopped status, and the doStart and StartAsync status writes cannot move a manager out of it. doStart's own failure path uses stopProcess, which leaves the status alone so the start error still reaches the health check.
• IsRunning and the started flag are removed. Only tests read them, and they could disagree with Status.

Scope

• A code-server that dies after it is ready still reports ready. That predates this PR and is not in it.
• If the exit cannot be watched (for example kqueue fails with EMFILE), the reaper falls back to a plain Wait, and Stop kills the leader through its handle only. The group is not swept on that path.

Tradeoffs

• idle, which the issue suggested, would let a health check racing TunnelClient.Close call StartAsync, which leaves a manager whose Start already ran stuck in starting. The new stopped value maps to HTTP 500 through the existing default branch, which the web editor already treats as an error.
• A child still handling the SIGTERM from Stop gets SIGKILL once the leader exits, so it no longer gets the rest of the grace period.

Blast radius

This touches only the code-server editor tunnel teardown. Two security reviews of the kill path found no issue that blocks the merge. The fixes for what they did find are in this PR: a start error that was lost, and a Stop that could block when the exit cannot be watched.

Verification

• The test commit comes first. On macOS the spawn-window and exited-leader tests time out with the group still running, and the status test reads ready after Stop.
• Each new ordering test fails against a reaper changed to reap first, or to hold the lock while it waits on a running leader.
• go test -race ./pkg/runner passes on macOS and on Linux (golang:1.26 in Docker), and the teardown tests pass 30 times in a row on both. go vet passes with GOOS=windows, and cmd/alpamon builds for every target in .goreleaser.yaml.

[Copilot]

🟡 Changes recommended

Moderate review findings require tests that exercise the actual startup failure and readiness race paths.

1 open issue

• Test asynchronous start failure status handling · New

What changed in this PR

This pull request closes code-server teardown gaps by cleaning up entire process groups after leader exit and making the manager’s stopped state terminal during concurrent startup operations.

Changes:

• Kill remaining process-group members after the leader is reaped.
• Prevent startup status updates from overwriting stopped.
• Remove redundant running-state tracking.
• Add Unix regression tests for teardown and status transitions.
• Review identified missing coverage for startup failure and readiness races.

File	Description
pkg/​runner/​editor.go	Implements post-reap process-group cleanup and terminal stopped-state handling.
pkg/​runner/​editor_unix_test.go	Adds and updates Unix teardown and manager-state regression tests.

_{Review effort: Lite}

[alpacax-wall-d]

WALL-D review: Approval withheld · head 7681fc0

Code review result: Human approval required · head 7681fc0

Review summary

A maintainer will review this PR.

Re-review requests and limits

_{WALL-D reviews a PR opened ready after this rule took effect once on its own. After that it reviews only on request: comment @alpacax-wall-d review or mark a draft ready for review, up to 5 times per PR including the first review. A re-review reads only the lines changed since the last review, and from the second re-review on only P1 findings block approval. Answering the product review's questions reviews only the product side again.}

[Jisung Chae (jisung-02)]

@alpacax-wall-d review

[alpacax-wall-d]

WALL-D review: Approved · head ec6b057 · no P1/P2, Product review result: Nothing flagged

[alpacax-wall-d]

Reviewed ec6b057: #600 (review)
#600 (comment)