Skip to content

Commit 1aa013a

Browse files
committed
fix(cli): use a readiness file instead of an inherited pipe fd
The session daemon spawn passed the readiness pipe to the detached child via cmd.ExtraFiles (fd 3), but os/exec's ExtraFiles is unsupported on Windows, so daemon.Start() failed with "fork/exec ...: not supported by windows" and the session never started there. Replace the inherited fd with a temp readiness file: the daemon writes its status atomically (write + rename) and `start` polls it until it sees a status, the daemon process exits, or a timeout slightly past the daemon's own agent-connect deadline. Works identically on Linux and Windows.
1 parent 94123b8 commit 1aa013a

2 files changed

Lines changed: 67 additions & 35 deletions

File tree

cmd/lk/session.go

Lines changed: 52 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
package main
1616

1717
import (
18-
"bufio"
1918
"context"
2019
"encoding/binary"
2120
"encoding/json"
@@ -26,6 +25,7 @@ import (
2625
"os/exec"
2726
"strconv"
2827
"strings"
28+
"time"
2929

3030
"github.com/urfave/cli/v3"
3131
)
@@ -38,11 +38,11 @@ const (
3838
sessionHost = "127.0.0.1"
3939
defaultSessionPort = 8775
4040

41-
envSessionPort = "LK_SESSION_PORT" // fixed port
42-
envSessionDir = "LK_SESSION_DIR" // resolved project dir
43-
envSessionEntry = "LK_SESSION_ENTRY" // resolved entrypoint (project-relative)
44-
envSessionPType = "LK_SESSION_PTYPE" // agentfs.ProjectType string
45-
envSessionReadyFD = "LK_SESSION_READY_FD"
41+
envSessionPort = "LK_SESSION_PORT" // fixed port
42+
envSessionDir = "LK_SESSION_DIR" // resolved project dir
43+
envSessionEntry = "LK_SESSION_ENTRY" // resolved entrypoint (project-relative)
44+
envSessionPType = "LK_SESSION_PTYPE" // agentfs.ProjectType string
45+
envSessionReadyFile = "LK_SESSION_READY_FILE" // path the daemon writes its status to
4646

4747
// sessionDaemonSubcommand is the hidden entrypoint `start` re-execs into.
4848
sessionDaemonSubcommand = "daemon"
@@ -92,7 +92,7 @@ var agentSessionCommand = &cli.Command{
9292
Name: sessionDaemonSubcommand,
9393
Hidden: true,
9494
Action: func(ctx context.Context, cmd *cli.Command) error {
95-
if os.Getenv(envSessionReadyFD) == "" {
95+
if os.Getenv(envSessionReadyFile) == "" {
9696
return fmt.Errorf("`session daemon` is an internal entrypoint; run `lk agent session start <entrypoint>` instead")
9797
}
9898
runSessionDaemon()
@@ -118,19 +118,20 @@ func runSessionStart(ctx context.Context, cmd *cli.Command) error {
118118
return fmt.Errorf("could not resolve own binary: %w", err)
119119
}
120120

121-
// Pipe the daemon uses to report readiness (or a startup error) before we
122-
// return. This avoids racing a TCP probe against the agent's own connect.
123-
readyR, readyW, err := os.Pipe()
121+
// Readiness file the daemon writes once it is up (or failed) before we
122+
// return, so we don't race a TCP probe against the agent's own connect.
123+
readyFile, err := os.CreateTemp("", "lk-session-ready-*.txt")
124124
if err != nil {
125125
return err
126126
}
127-
defer readyR.Close()
127+
readyPath := readyFile.Name()
128+
readyFile.Close()
129+
defer os.Remove(readyPath)
128130

129131
// The daemon is detached, so its own stdout/stderr (panics etc.) go to a
130132
// temp log rather than the user's terminal.
131133
logFile, err := os.CreateTemp("", "lk-session-daemon-*.log")
132134
if err != nil {
133-
readyW.Close()
134135
return err
135136
}
136137

@@ -140,24 +141,19 @@ func runSessionStart(ctx context.Context, cmd *cli.Command) error {
140141
envSessionDir+"="+projectDir,
141142
envSessionEntry+"="+entrypoint,
142143
envSessionPType+"="+string(projectType),
143-
envSessionReadyFD+"=3", // ExtraFiles[0] is fd 3 in the child
144+
envSessionReadyFile+"="+readyPath,
144145
)
145-
daemon.ExtraFiles = []*os.File{readyW}
146146
daemon.Stdout = logFile
147147
daemon.Stderr = logFile
148148
setDetachedProcAttr(daemon)
149149

150150
if err := daemon.Start(); err != nil {
151-
readyW.Close()
152151
logFile.Close()
153152
return fmt.Errorf("failed to start session daemon: %w", err)
154153
}
155-
// Close our copy of the write end so the read below sees EOF if the daemon dies.
156-
readyW.Close()
157154
logFile.Close()
158155

159-
status, _ := bufio.NewReader(readyR).ReadString('\n')
160-
status = strings.TrimSpace(status)
156+
status := awaitDaemonReady(daemon, readyPath)
161157
switch {
162158
case status == "ready":
163159
fmt.Fprintf(os.Stderr, "Detected %s agent (%s in %s)\n", projectType.Lang(), entrypoint, projectDir)
@@ -170,6 +166,43 @@ func runSessionStart(ctx context.Context, cmd *cli.Command) error {
170166
}
171167
}
172168

169+
// awaitDaemonReady waits for the detached daemon to report via the readiness
170+
// file, returning its status line ("ready" or "error: ...") or "" if the
171+
// daemon exits or times out without reporting.
172+
func awaitDaemonReady(daemon *exec.Cmd, readyPath string) string {
173+
exited := make(chan struct{})
174+
go func() { _ = daemon.Wait(); close(exited) }()
175+
176+
// Slightly longer than the daemon's own 60s agent-connect timeout so its
177+
// "error: timed out ..." status reaches us before we give up.
178+
timeout := time.After(65 * time.Second)
179+
for {
180+
if status, ok := readReadyStatus(readyPath); ok {
181+
return status
182+
}
183+
select {
184+
case <-exited:
185+
if status, ok := readReadyStatus(readyPath); ok {
186+
return status
187+
}
188+
return ""
189+
case <-timeout:
190+
return ""
191+
case <-time.After(50 * time.Millisecond):
192+
}
193+
}
194+
}
195+
196+
// readReadyStatus returns the daemon's status line once the readiness file has
197+
// content (written atomically via rename), or ok=false while it is still empty.
198+
func readReadyStatus(path string) (string, bool) {
199+
data, err := os.ReadFile(path)
200+
if err != nil || len(data) == 0 {
201+
return "", false
202+
}
203+
return strings.TrimSpace(string(data)), true
204+
}
205+
173206
func runSessionSay(ctx context.Context, cmd *cli.Command) error {
174207
text := strings.TrimSpace(strings.Join(cmd.Args().Slice(), " "))
175208
if text == "" {

cmd/lk/session_daemon.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,26 +100,25 @@ func runSessionDaemon() {
100100
agentProc.Kill()
101101
}
102102

103-
// readyWriter returns the inherited pipe `lk agent session start` reads to learn the
104-
// daemon became ready (or failed). Nil if not launched via start.
105-
func readyWriter() *os.File {
106-
fdStr := os.Getenv(envSessionReadyFD)
107-
if fdStr == "" {
108-
return nil
109-
}
110-
fd, err := strconv.Atoi(fdStr)
111-
if err != nil {
112-
return nil
113-
}
114-
return os.NewFile(uintptr(fd), "ready")
103+
// readyWriter returns the path of the readiness file `lk agent session start`
104+
// polls to learn the daemon became ready (or failed). Empty if not launched
105+
// via start.
106+
func readyWriter() string {
107+
return os.Getenv(envSessionReadyFile)
115108
}
116109

117-
func signalReady(f *os.File, msg string) {
118-
if f == nil {
110+
// signalReady atomically writes the daemon's status to the readiness file the
111+
// parent `start` is polling. The write-then-rename keeps the parent from
112+
// reading a partial line.
113+
func signalReady(path, msg string) {
114+
if path == "" {
115+
return
116+
}
117+
tmp := path + ".tmp"
118+
if err := os.WriteFile(tmp, []byte(msg+"\n"), 0o600); err != nil {
119119
return
120120
}
121-
fmt.Fprintln(f, msg)
122-
f.Close()
121+
_ = os.Rename(tmp, path)
123122
}
124123

125124
type sessionDaemon struct {

0 commit comments

Comments
 (0)