From b189c7520e2f780e44c530d01bce77cfaaa24eff Mon Sep 17 00:00:00 2001 From: kjh0718 Date: Mon, 7 Sep 2026 01:06:13 +0900 Subject: [PATCH 1/7] =?UTF-8?q?refactor:=20interactive=20terminal=20sessio?= =?UTF-8?q?n=20=EC=B6=94=EC=83=81=ED=99=94=20(#1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude/Codex interactive trigger의 PTY 및 process lifecycle을 internal/terminal.Session으로 추상화한다. - Unix/macOS는 기존 creack/pty 동작 유지 - Windows는 현재 unsupported stub 유지 - provider의 직접 PTY 의존 제거 - terminal abstraction 최소 테스트 추가 ConPTY backend와 Windows continue proxy는 후속 작업으로 분리한다. Refs #1 --- internal/provider/claude.go | 43 ++++---- internal/provider/codex.go | 42 ++++---- internal/terminal/session.go | 37 +++++++ internal/terminal/session_unix.go | 42 ++++++++ internal/terminal/session_unix_test.go | 116 ++++++++++++++++++++++ internal/terminal/session_windows.go | 17 ++++ internal/terminal/session_windows_test.go | 47 +++++++++ 7 files changed, 295 insertions(+), 49 deletions(-) create mode 100644 internal/terminal/session.go create mode 100644 internal/terminal/session_unix.go create mode 100644 internal/terminal/session_unix_test.go create mode 100644 internal/terminal/session_windows.go create mode 100644 internal/terminal/session_windows_test.go diff --git a/internal/provider/claude.go b/internal/provider/claude.go index 11f825e..cd4dc4d 100644 --- a/internal/provider/claude.go +++ b/internal/provider/claude.go @@ -7,18 +7,16 @@ import ( "fmt" "io" "net/http" - "os" "os/exec" "regexp" "strings" "sync" "time" - "github.com/creack/pty" - "github.com/wavever/CCLimitPing/internal/activity" "github.com/wavever/CCLimitPing/internal/auth" "github.com/wavever/CCLimitPing/internal/config" + "github.com/wavever/CCLimitPing/internal/terminal" "github.com/wavever/CCLimitPing/internal/usage" ) @@ -272,58 +270,55 @@ func (c *Claude) Trigger(ctx context.Context, dryRun bool) (*TriggerResult, erro return res, nil } - cmd := exec.CommandContext(ctx, "claude", args...) - ptmx, err := pty.Start(cmd) + sess, err := terminal.Start(ctx, "claude", args) if err != nil { return res, fmt.Errorf("claude interactive failed to start: %w", err) } - defer ptmx.Close() + defer sess.Close() output := &limitedBuffer{limit: 4096} go func() { - _, _ = io.Copy(output, ptmx) + _, _ = io.Copy(output, sess) }() done := make(chan error, 1) go func() { - done <- cmd.Wait() + done <- sess.Wait() }() // Phase 1: wait for the TUI to render and settle so the submit Enter lands on // a ready prompt holding the prefilled message. - if terminal, err := claudeAwait(ctx, cmd, ptmx, output, done, claudeStartupTimeout, - func(idle, _ time.Duration) bool { return idle >= claudeStartupSettle }); terminal { + if term, err := claudeAwait(ctx, sess, output, done, claudeStartupTimeout, + func(idle, _ time.Duration) bool { return idle >= claudeStartupSettle }); term { return res, err } // Submit the prefilled prompt. This is the model request that anchors the 5h // window; the previous implementation never sent it, so the window never // started even though the session exited cleanly. - if _, werr := ptmx.Write([]byte("\r")); werr != nil { + if _, werr := sess.Write([]byte("\r")); werr != nil { return res, fmt.Errorf("claude interactive failed to submit prompt: %w: %s", werr, truncate(output.Bytes(), 300)) } // Phase 2: let the turn run until its output goes quiet (bounded by a floor // and a hard cap), so we don't cancel the in-flight request by exiting early. - if terminal, err := claudeAwait(ctx, cmd, ptmx, output, done, claudeTurnMaxWait, + if term, err := claudeAwait(ctx, sess, output, done, claudeTurnMaxWait, func(idle, elapsed time.Duration) bool { return elapsed >= claudeTurnMinWait && idle >= claudeTurnQuiet - }); terminal { + }); term { return res, err } // Phase 3: quit. The window is already anchored, so a messy shutdown here // must not fail the ping. - _, _ = ptmx.Write([]byte("/exit\r")) + _, _ = sess.Write([]byte("/exit\r")) select { case err := <-done: return res, claudeInteractiveErr(err, output) case <-ctx.Done(): - return res, claudeInteractiveCancel(ctx, cmd, ptmx, done, output) + return res, claudeInteractiveCancel(ctx, sess, done, output) case <-time.After(claudeExitGrace): - if cmd.Process != nil { - _ = cmd.Process.Kill() - } + _ = sess.Kill() select { case <-done: case <-time.After(time.Second): @@ -340,7 +335,7 @@ func (c *Claude) Trigger(ctx context.Context, dryRun bool) (*TriggerResult, erro // PTY output and elapsed is the time since this phase began. It returns // terminal=true (with an error to propagate) only if the process exits or ctx is // cancelled first; otherwise terminal=false and the caller continues. -func claudeAwait(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, output *limitedBuffer, done <-chan error, maxWait time.Duration, ready func(idle, elapsed time.Duration) bool) (bool, error) { +func claudeAwait(ctx context.Context, sess terminal.Session, output *limitedBuffer, done <-chan error, maxWait time.Duration, ready func(idle, elapsed time.Duration) bool) (bool, error) { start := time.Now() deadline := time.After(maxWait) ticker := time.NewTicker(claudePollInterval) @@ -350,7 +345,7 @@ func claudeAwait(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, output *limi case err := <-done: return true, claudeInteractiveErr(err, output) case <-ctx.Done(): - return true, claudeInteractiveCancel(ctx, cmd, ptmx, done, output) + return true, claudeInteractiveCancel(ctx, sess, done, output) case <-deadline: return false, nil case <-ticker.C: @@ -386,11 +381,9 @@ func claudeSubscriptionErrorFromOutput(raw []byte) error { return nil } -func claudeInteractiveCancel(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, done <-chan error, output *limitedBuffer) error { - if cmd.Process != nil { - _ = cmd.Process.Kill() - } - _ = ptmx.Close() +func claudeInteractiveCancel(ctx context.Context, sess terminal.Session, done <-chan error, output *limitedBuffer) error { + _ = sess.Kill() + _ = sess.Close() select { case <-done: case <-time.After(time.Second): diff --git a/internal/provider/codex.go b/internal/provider/codex.go index 7fb0c45..a15d50c 100644 --- a/internal/provider/codex.go +++ b/internal/provider/codex.go @@ -12,7 +12,6 @@ import ( "net/http" "net/url" "os" - "os/exec" "path/filepath" "strings" "sync" @@ -20,11 +19,11 @@ import ( "unicode" "github.com/BurntSushi/toml" - "github.com/creack/pty" "github.com/wavever/CCLimitPing/internal/activity" "github.com/wavever/CCLimitPing/internal/auth" "github.com/wavever/CCLimitPing/internal/config" + "github.com/wavever/CCLimitPing/internal/terminal" "github.com/wavever/CCLimitPing/internal/usage" ) @@ -569,34 +568,33 @@ func triggerCodex(ctx context.Context, cfg config.ProviderConfig, dryRun bool) ( return res, nil } - cmd := exec.CommandContext(ctx, "codex", args...) - ptmx, err := pty.Start(cmd) + sess, err := terminal.Start(ctx, "codex", args) if err != nil { return res, fmt.Errorf("codex interactive failed to start: %w", err) } - defer ptmx.Close() + defer sess.Close() output := &limitedBuffer{limit: 4096} go func() { - _, _ = io.Copy(output, ptmx) + _, _ = io.Copy(output, sess) }() done := make(chan error, 1) go func() { - done <- cmd.Wait() + done <- sess.Wait() }() - if terminal, err := codexAwait(ctx, cmd, ptmx, output, done, codexTurnMaxWait, + if term, err := codexAwait(ctx, sess, output, done, codexTurnMaxWait, func(idle, elapsed time.Duration) bool { return elapsed >= codexTurnMinWait && idle >= codexTurnQuiet - }); terminal { + }); term { return res, err } - return res, codexInteractiveStop(ctx, cmd, ptmx, done, output) + return res, codexInteractiveStop(ctx, sess, done, output) } -func codexAwait(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, output *limitedBuffer, done <-chan error, maxWait time.Duration, ready func(idle, elapsed time.Duration) bool) (bool, error) { +func codexAwait(ctx context.Context, sess terminal.Session, output *limitedBuffer, done <-chan error, maxWait time.Duration, ready func(idle, elapsed time.Duration) bool) (bool, error) { start := time.Now() deadline := time.After(maxWait) ticker := time.NewTicker(codexPollInterval) @@ -606,7 +604,7 @@ func codexAwait(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, output *limit case err := <-done: return true, codexInteractiveErr(err, output) case <-ctx.Done(): - return true, codexInteractiveCancel(ctx, cmd, ptmx, done, output) + return true, codexInteractiveCancel(ctx, sess, done, output) case <-deadline: return false, nil case <-ticker.C: @@ -618,27 +616,25 @@ func codexAwait(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, output *limit } } -func codexInteractiveStop(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, done <-chan error, output *limitedBuffer) error { +func codexInteractiveStop(ctx context.Context, sess terminal.Session, done <-chan error, output *limitedBuffer) error { deadline := time.After(codexExitGrace) ticker := time.NewTicker(codexExitGrace / 2) defer ticker.Stop() for sent := false; ; { if !sent { - _, _ = ptmx.Write([]byte{0x03}) + _, _ = sess.Write([]byte{0x03}) sent = true } select { case <-done: return nil case <-ctx.Done(): - return codexInteractiveCancel(ctx, cmd, ptmx, done, output) + return codexInteractiveCancel(ctx, sess, done, output) case <-ticker.C: - _, _ = ptmx.Write([]byte{0x03}) + _, _ = sess.Write([]byte{0x03}) case <-deadline: - if cmd.Process != nil { - _ = cmd.Process.Kill() - } + _ = sess.Kill() select { case <-done: case <-time.After(time.Second): @@ -659,11 +655,9 @@ func codexInteractiveErr(err error, output *limitedBuffer) error { return fmt.Errorf("codex interactive failed: %w: %s", err, tail) } -func codexInteractiveCancel(ctx context.Context, cmd *exec.Cmd, ptmx *os.File, done <-chan error, output *limitedBuffer) error { - if cmd.Process != nil { - _ = cmd.Process.Kill() - } - _ = ptmx.Close() +func codexInteractiveCancel(ctx context.Context, sess terminal.Session, done <-chan error, output *limitedBuffer) error { + _ = sess.Kill() + _ = sess.Close() select { case <-done: case <-time.After(time.Second): diff --git a/internal/terminal/session.go b/internal/terminal/session.go new file mode 100644 index 0000000..0465d0d --- /dev/null +++ b/internal/terminal/session.go @@ -0,0 +1,37 @@ +// Package terminal abstracts launching a child process attached to a +// pseudo-terminal, so provider triggers can drive an interactive CLI (Claude +// Code, Codex) without depending on a specific PTY backend. Unix/macOS uses a +// real PTY (github.com/creack/pty); Windows is not supported yet (a ConPTY +// backend is tracked separately) and returns an error from Start. +package terminal + +import ( + "context" + "io" +) + +// Session is a child process running under a pseudo-terminal. Read and Write +// are the PTY master side — Read drains the child's output, Write feeds its +// input. Close tears the PTY down, Wait blocks until the child exits, and Kill +// force-terminates it. The abstraction owns the whole process lifecycle (not +// just the I/O stream) because a PTY backend may spawn the child itself rather +// than through os/exec, so Wait and Kill cannot be assumed to come from an +// externally held *exec.Cmd. +type Session interface { + io.Reader + io.Writer + io.Closer + // Wait blocks until the child exits, returning its exit error (nil on a + // clean exit), matching (*exec.Cmd).Wait semantics. + Wait() error + // Kill force-terminates the child. It is safe to call when the process is + // already gone. + Kill() error +} + +// Start launches name (with args) attached to a new pseudo-terminal and returns +// a Session that owns the child's lifecycle. Cancelling ctx terminates the +// child. On platforms without PTY support it returns an error and a nil Session. +func Start(ctx context.Context, name string, args []string) (Session, error) { + return start(ctx, name, args) +} diff --git a/internal/terminal/session_unix.go b/internal/terminal/session_unix.go new file mode 100644 index 0000000..c237e79 --- /dev/null +++ b/internal/terminal/session_unix.go @@ -0,0 +1,42 @@ +//go:build !windows + +package terminal + +import ( + "context" + "os" + "os/exec" + + "github.com/creack/pty" +) + +// unixSession wraps a creack/pty master file and the exec.Cmd running under it. +// This preserves the exact behavior the provider triggers relied on before the +// terminal seam was introduced: exec.CommandContext binds ctx cancellation to +// the child, pty.Start attaches the PTY, and the master file is the child's +// stdin/stdout. +type unixSession struct { + cmd *exec.Cmd + ptmx *os.File +} + +func start(ctx context.Context, name string, args []string) (Session, error) { + cmd := exec.CommandContext(ctx, name, args...) + ptmx, err := pty.Start(cmd) + if err != nil { + return nil, err + } + return &unixSession{cmd: cmd, ptmx: ptmx}, nil +} + +func (s *unixSession) Read(p []byte) (int, error) { return s.ptmx.Read(p) } +func (s *unixSession) Write(p []byte) (int, error) { return s.ptmx.Write(p) } +func (s *unixSession) Close() error { return s.ptmx.Close() } +func (s *unixSession) Wait() error { return s.cmd.Wait() } + +func (s *unixSession) Kill() error { + if s.cmd.Process == nil { + return nil + } + return s.cmd.Process.Kill() +} diff --git a/internal/terminal/session_unix_test.go b/internal/terminal/session_unix_test.go new file mode 100644 index 0000000..1c5040b --- /dev/null +++ b/internal/terminal/session_unix_test.go @@ -0,0 +1,116 @@ +//go:build !windows + +package terminal + +import ( + "bytes" + "context" + "strings" + "sync" + "testing" + "time" +) + +// TestStartRunsCommandAndReadsOutput starts a trivial shell command under a real +// PTY, reads its output through the Session, and confirms Wait returns cleanly. +// It uses `sh -c echo` (always present on Unix), never a real CLI or credentials. +func TestStartRunsCommandAndReadsOutput(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + const marker = "pty-test-output" + sess, err := Start(ctx, "sh", []string{"-c", "echo " + marker}) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer sess.Close() + + // Drain the PTY master concurrently. Reading a master after the child exits + // can return EIO on Linux (vs a clean EOF on macOS), so the read error is + // intentionally ignored — the assertion is on the captured bytes. + var ( + mu sync.Mutex + buf bytes.Buffer + ) + readDone := make(chan struct{}) + go func() { + defer close(readDone) + b := make([]byte, 1024) + for { + n, rerr := sess.Read(b) + if n > 0 { + mu.Lock() + buf.Write(b[:n]) + mu.Unlock() + } + if rerr != nil { + return + } + } + }() + + waitErr := make(chan error, 1) + go func() { waitErr <- sess.Wait() }() + + select { + case err := <-waitErr: + if err != nil { + t.Fatalf("Wait returned an error for a clean exit: %v", err) + } + case <-ctx.Done(): + _ = sess.Kill() + t.Fatalf("timed out waiting for the command: %v", ctx.Err()) + } + + // Closing the master unblocks the reader if the child's exit didn't. + _ = sess.Close() + select { + case <-readDone: + case <-time.After(2 * time.Second): + t.Fatal("reader did not finish after Close (possible hang)") + } + + mu.Lock() + got := buf.String() + mu.Unlock() + if !strings.Contains(got, marker) { + t.Fatalf("PTY output %q does not contain %q", got, marker) + } +} + +// TestKillTerminatesCommand confirms Kill stops a long-running child and that +// Wait then returns, all within the context deadline (no hang, no leaked child). +func TestKillTerminatesCommand(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + sess, err := Start(ctx, "sh", []string{"-c", "sleep 60"}) + if err != nil { + t.Fatalf("Start: %v", err) + } + defer sess.Close() + + // Keep the master drained so the child never blocks on a full PTY buffer. + go func() { + b := make([]byte, 512) + for { + if _, rerr := sess.Read(b); rerr != nil { + return + } + } + }() + + if err := sess.Kill(); err != nil { + t.Fatalf("Kill: %v", err) + } + + waitErr := make(chan error, 1) + go func() { waitErr <- sess.Wait() }() + select { + case <-waitErr: + // A killed process makes Wait return a non-nil error; either way it + // must return, which is what this test asserts. + case <-ctx.Done(): + t.Fatalf("Wait did not return after Kill (possible hang): %v", ctx.Err()) + } +} diff --git a/internal/terminal/session_windows.go b/internal/terminal/session_windows.go new file mode 100644 index 0000000..5786a88 --- /dev/null +++ b/internal/terminal/session_windows.go @@ -0,0 +1,17 @@ +//go:build windows + +package terminal + +import ( + "context" + "errors" +) + +// ErrUnsupported is returned by Start on Windows until the ConPTY backend lands. +// The interactive provider triggers require a real pseudo-terminal to anchor the +// subscription window, which github.com/creack/pty does not provide on Windows. +var ErrUnsupported = errors.New("interactive PTY sessions are not supported on Windows yet") + +func start(_ context.Context, _ string, _ []string) (Session, error) { + return nil, ErrUnsupported +} diff --git a/internal/terminal/session_windows_test.go b/internal/terminal/session_windows_test.go new file mode 100644 index 0000000..3708683 --- /dev/null +++ b/internal/terminal/session_windows_test.go @@ -0,0 +1,47 @@ +//go:build windows + +package terminal + +import ( + "context" + "errors" + "testing" + "time" +) + +// TestStartUnsupportedOnWindows verifies the current Windows behavior: Start +// reports ErrUnsupported, returns a nil Session, and does so promptly (no hang). +// The ConPTY backend will replace this stub; until then this pins the contract. +func TestStartUnsupportedOnWindows(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + type result struct { + sess Session + err error + } + done := make(chan result, 1) + go func() { + // A harmless command line; it must never actually run on the stub path. + sess, err := Start(ctx, "cmd", []string{"/c", "echo hi"}) + done <- result{sess: sess, err: err} + }() + + select { + case r := <-done: + if r.err == nil { + if r.sess != nil { + _ = r.sess.Close() + } + t.Fatal("Start returned a nil error on Windows; expected unsupported") + } + if !errors.Is(r.err, ErrUnsupported) { + t.Fatalf("Start error = %v, want ErrUnsupported", r.err) + } + if r.sess != nil { + t.Fatalf("Start returned a non-nil Session on Windows: %v", r.sess) + } + case <-time.After(2 * time.Second): + t.Fatal("Start did not return promptly on Windows (possible hang)") + } +} From 9c7bec2328edbb542e181aa20a8a75509ea29e31 Mon Sep 17 00:00:00 2001 From: kjh0718 Date: Mon, 7 Sep 2026 01:29:06 +0900 Subject: [PATCH 2/7] =?UTF-8?q?test:=20PTY=20session=EC=9D=84=20=EC=A0=95?= =?UTF-8?q?=ED=99=95=ED=9E=88=20=ED=95=9C=20=EB=B2=88=EB=A7=8C=20Close?= =?UTF-8?q?=ED=95=98=EB=8F=84=EB=A1=9D=20=EC=A0=95=EB=A6=AC=20(#1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit PR #2 리뷰를 반영해 Unix PTY 테스트의 cleanup을 정리한다. - 같은 Session이 두 번 Close되지 않도록 guard 적용 - Close() error를 무시하지 않고 테스트 실패로 보고 - 조기 실패 경로는 t.Cleanup으로 안전하게 정리 production 코드 변경 없음. Refs #1 --- internal/terminal/session_unix_test.go | 29 +++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/internal/terminal/session_unix_test.go b/internal/terminal/session_unix_test.go index 1c5040b..7fed5fc 100644 --- a/internal/terminal/session_unix_test.go +++ b/internal/terminal/session_unix_test.go @@ -23,7 +23,23 @@ func TestStartRunsCommandAndReadsOutput(t *testing.T) { if err != nil { t.Fatalf("Start: %v", err) } - defer sess.Close() + // Close the PTY session exactly once. The guard keeps the in-body close + // (which unblocks the reader) and the cleanup safety net mutually + // exclusive, so the session is never closed twice; the close error is + // reported at the in-body call rather than dropped. + closed := false + closeSession := func() error { + if closed { + return nil + } + closed = true + return sess.Close() + } + t.Cleanup(func() { + if err := closeSession(); err != nil { + t.Errorf("Close: %v", err) + } + }) // Drain the PTY master concurrently. Reading a master after the child exits // can return EIO on Linux (vs a clean EOF on macOS), so the read error is @@ -63,7 +79,9 @@ func TestStartRunsCommandAndReadsOutput(t *testing.T) { } // Closing the master unblocks the reader if the child's exit didn't. - _ = sess.Close() + if err := closeSession(); err != nil { + t.Errorf("Close: %v", err) + } select { case <-readDone: case <-time.After(2 * time.Second): @@ -88,7 +106,12 @@ func TestKillTerminatesCommand(t *testing.T) { if err != nil { t.Fatalf("Start: %v", err) } - defer sess.Close() + // Single, error-checked close (no second Close call anywhere in this test). + defer func() { + if err := sess.Close(); err != nil { + t.Errorf("Close: %v", err) + } + }() // Keep the master drained so the child never blocks on a full PTY buffer. go func() { From adc0b38fd91c43d143dbc55c191265182cc14de1 Mon Sep 17 00:00:00 2001 From: kjh0718 Date: Mon, 7 Sep 2026 17:32:40 +0900 Subject: [PATCH 3/7] =?UTF-8?q?feat:=20Windows=20ConPTY=20terminal=20backe?= =?UTF-8?q?nd=20=EC=B6=94=EA=B0=80=20(#1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows에서도 interactive provider trigger를 실행할 수 있도록 golang.org/x/sys/windows 기반 ConPTY backend를 구현한다. - CreatePseudoConsole과 PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE로 interactive child를 pseudoconsole에 연결 - CREATE_SUSPENDED → Job Object 할당 → ResumeThread 순서로 descendant process의 job escape race 차단 - background 실행 환경에서도 child stdio가 ConPTY로 연결되도록 STARTF_USESTDHANDLES와 NULL standard handles 적용 - raw ReadFile/WriteFile로 ConPTY I/O 처리 - child 종료 시 HPCON을 비동기로 닫아 blocked Read의 EOF 보장 - Job Object와 conhost bounded reap으로 process tree와 console host 정리 - Wait/Kill/Close/context cancellation lifecycle 동기화 - Windows lifecycle, stdin round-trip, process tree, conhost, Ctrl-C, 반복/동시 session 테스트 추가 Refs #1 --- go.mod | 2 +- internal/terminal/conpty_windows.go | 348 ++++++++++++++ internal/terminal/jobobject_windows.go | 92 ++++ internal/terminal/session.go | 8 +- internal/terminal/session_windows.go | 230 +++++++++- internal/terminal/session_windows_test.go | 534 +++++++++++++++++++++- 6 files changed, 1179 insertions(+), 35 deletions(-) create mode 100644 internal/terminal/conpty_windows.go create mode 100644 internal/terminal/jobobject_windows.go diff --git a/go.mod b/go.mod index a92bd5b..0e7899c 100644 --- a/go.mod +++ b/go.mod @@ -6,11 +6,11 @@ require ( github.com/BurntSushi/toml v1.6.0 github.com/creack/pty v1.1.24 github.com/spf13/cobra v1.10.2 + golang.org/x/sys v0.46.0 golang.org/x/term v0.44.0 ) require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/spf13/pflag v1.0.9 // indirect - golang.org/x/sys v0.46.0 // indirect ) diff --git a/internal/terminal/conpty_windows.go b/internal/terminal/conpty_windows.go new file mode 100644 index 0000000..2611e9f --- /dev/null +++ b/internal/terminal/conpty_windows.go @@ -0,0 +1,348 @@ +//go:build windows + +package terminal + +import ( + "fmt" + "io" + "os" + "strings" + "sync" + "time" + "unsafe" + + "golang.org/x/sys/windows" +) + +// conPTYSize is the size every pseudoconsole is created with. The interactive +// CLIs only need a plausible terminal to lay their TUI out in, and nothing here +// resizes, so this is fixed rather than tracked against a real console: the ping +// paths run detached, where there is no console to track. +var conPTYSize = windows.Coord{X: 120, Y: 30} + +const ( + // conhostExitTimeout bounds how long teardown waits for the console host to + // run itself down after its clients are gone, before reaping it. + conhostExitTimeout = time.Second + // hpconCloseTimeout bounds how long teardown waits for the background + // ClosePseudoConsole to return before leaving it to finish on its own. + hpconCloseTimeout = 2 * time.Second +) + +// pipeHandle owns one end of an anonymous pipe and serializes closing the +// handle against the I/O in flight on it. +// +// A blocked ReadFile is unblocked by the pipe breaking — the ConPTY side goes +// away once the pseudoconsole is closed and the console host exits — and not by +// closing the handle underneath the reader: Win32 does not define closing a +// handle that a synchronous ReadFile is parked on, and the freed handle value +// can be reused by the next CreatePipe in this process, which would leave a +// still-live read pointed at an unrelated pipe. So Close only marks the end +// closed and leaves the actual CloseHandle to whichever side finishes last. +type pipeHandle struct { + mu sync.Mutex + h windows.Handle + busy int + closed bool + freed bool +} + +func newPipeHandle(h windows.Handle) *pipeHandle { return &pipeHandle{h: h} } + +// acquire pins the handle for one syscall, or reports that the end is closed. +func (p *pipeHandle) acquire() (windows.Handle, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.closed { + return 0, os.ErrClosed + } + p.busy++ + return p.h, nil +} + +func (p *pipeHandle) release() { + p.mu.Lock() + defer p.mu.Unlock() + p.busy-- + p.freeLocked() +} + +// freeLocked closes the handle once the end is closed and no syscall is holding +// it. Callers hold p.mu. +func (p *pipeHandle) freeLocked() { + if p.freed || !p.closed || p.busy > 0 { + return + } + p.freed = true + _ = windows.CloseHandle(p.h) +} + +// Close marks the end closed. It is idempotent and never blocks on in-flight +// I/O; see the type comment for who runs the CloseHandle. +func (p *pipeHandle) Close() error { + p.mu.Lock() + defer p.mu.Unlock() + p.closed = true + p.freeLocked() + return nil +} + +// Read reads from the pipe with a raw ReadFile rather than wrapping the handle +// in an *os.File: os.NewFile hands the handle to the runtime poller, which does +// not fit a synchronous anonymous pipe, and reads past the pseudoconsole's +// opening handshake then stall indefinitely even while the child keeps writing. +// Owning the syscall also lets the pipe's end states map onto io.EOF exactly. +func (p *pipeHandle) Read(b []byte) (int, error) { + if len(b) == 0 { + return 0, nil + } + h, err := p.acquire() + if err != nil { + // Reading a torn-down session is end of input, not a failure: the + // providers drain the session with io.Copy while closing it elsewhere. + return 0, io.EOF + } + defer p.release() + + var n uint32 + if err := windows.ReadFile(h, b, &n, nil); err != nil { + if isPipeEOF(err) { + return int(n), io.EOF + } + return int(n), err + } + if n == 0 { + // A zero-length read on a byte pipe only happens at end of input. + return 0, io.EOF + } + return int(n), nil +} + +// Write feeds the child's stdin, looping over short writes as io.Writer wants. +func (p *pipeHandle) Write(b []byte) (int, error) { + if len(b) == 0 { + return 0, nil + } + h, err := p.acquire() + if err != nil { + return 0, err + } + defer p.release() + + total := 0 + for total < len(b) { + var n uint32 + if err := windows.WriteFile(h, b[total:], &n, nil); err != nil { + if isPipeEOF(err) { + return total + int(n), io.ErrClosedPipe + } + return total + int(n), err + } + if n == 0 { + return total, io.ErrShortWrite + } + total += int(n) + } + return total, nil +} + +// isPipeEOF reports whether err means the far end of the pipe is gone, which is +// the normal way a ConPTY session ends: the console host closes its side when +// the pseudoconsole is closed after the child exits. +func isPipeEOF(err error) bool { + switch err { + case windows.ERROR_BROKEN_PIPE, windows.ERROR_PIPE_NOT_CONNECTED, windows.ERROR_HANDLE_EOF: + return true + case windows.ERROR_OPERATION_ABORTED, windows.ERROR_INVALID_HANDLE: + // The session was torn down underneath the call. + return true + } + return false +} + +// conPTY is a pseudoconsole plus the parent-side ends of the pipes wired to it: +// in feeds the child's stdin, out drains everything the console renders. +type conPTY struct { + hpc windows.Handle + // conhost is a handle to the console host serving this pseudoconsole, or 0 + // when it could not be identified; see openNewConhostChild. + conhost windows.Handle + + in *pipeHandle + out *pipeHandle + + hpcOnce sync.Once + hpcDone chan struct{} +} + +// newConPTY creates a pseudoconsole of the given size and returns the parent +// side of it. +func newConPTY(size windows.Coord) (*conPTY, error) { + var inRead, inWrite, outRead, outWrite windows.Handle + if err := windows.CreatePipe(&inRead, &inWrite, nil, 0); err != nil { + return nil, fmt.Errorf("CreatePipe (stdin): %w", err) + } + if err := windows.CreatePipe(&outRead, &outWrite, nil, 0); err != nil { + _ = windows.CloseHandle(inRead) + _ = windows.CloseHandle(inWrite) + return nil, fmt.Errorf("CreatePipe (stdout): %w", err) + } + + // CreatePseudoConsole also spawns the conhost.exe serving the console + // session, as a direct child of this process. Teardown wants a handle to it + // (see reapConhost) but Windows offers no way to derive one from the HPCON, + // so identify it by diffing our conhost children across the call, under a + // lock so that two sessions starting at once cannot make each other's diff + // ambiguous. The handle is opened right away, before the pid can be reused. + var hpc, conhost windows.Handle + conhostScanMu.Lock() + before := conhostChildren() + err := windows.CreatePseudoConsole(size, inRead, outWrite, 0, &hpc) + if err == nil { + conhost = openNewConhostChild(before) + } + conhostScanMu.Unlock() + + // The pseudoconsole duplicates the child-side ends it needs, so drop ours + // either way: holding the output pipe's write end open here would keep the + // reader from ever seeing EOF. + _ = windows.CloseHandle(inRead) + _ = windows.CloseHandle(outWrite) + if err != nil { + _ = windows.CloseHandle(inWrite) + _ = windows.CloseHandle(outRead) + return nil, fmt.Errorf("CreatePseudoConsole: %w", err) + } + + return &conPTY{ + hpc: hpc, + conhost: conhost, + in: newPipeHandle(inWrite), + out: newPipeHandle(outRead), + hpcDone: make(chan struct{}), + }, nil +} + +// closeHPCON starts closing the pseudoconsole, exactly once, on its own +// goroutine, and returns a channel that is closed when that call returns. +// +// It must run off the caller's thread because ClosePseudoConsole can block for +// a long time: before Windows 11 24H2 it waits for the console host to exit, +// and closing only delivers CTRL_CLOSE_EVENT to the attached client rather than +// terminating it, so a client that keeps running keeps the host — and with it +// ClosePseudoConsole — alive arbitrarily long. Sequencing the job kill after a +// blocking close would deadlock in exactly the case the kill exists for, so +// teardown starts this and kills in parallel. +// +// Closing is also what ends the output stream: unlike a Unix master fd, which +// EOFs when the slave side closes on child exit, the ConPTY output pipe stays +// alive until the pseudoconsole is closed. The waiter therefore calls this as +// soon as the child exits, so a blocked Read sees EOF once the buffered output +// has drained. +func (c *conPTY) closeHPCON() <-chan struct{} { + c.hpcOnce.Do(func() { + go func() { + defer close(c.hpcDone) + windows.ClosePseudoConsole(c.hpc) + }() + }) + return c.hpcDone +} + +// reapConhost waits briefly for the console host to run itself down once its +// clients are gone, and terminates it if it does not. A healthy conhost exits by +// itself after the pseudoconsole is closed and its clients are gone, leaving +// this as just the handle close; conhost builds before Windows 11 24H2 can fail +// to complete that rundown when a client was killed after the close event was +// delivered — the fate of every client the job kill is for — and such a host +// then sits around forever serving nothing. +func (c *conPTY) reapConhost() { + if c.conhost == 0 { + return + } + event, err := windows.WaitForSingleObject(c.conhost, uint32(conhostExitTimeout/time.Millisecond)) + if err != nil || event != windows.WAIT_OBJECT_0 { + _ = windows.TerminateProcess(c.conhost, 1) + } + _ = windows.CloseHandle(c.conhost) + c.conhost = 0 +} + +// shutdown releases everything the pseudoconsole owns, running kill (when the +// caller has a child to reap) after the pseudoconsole close has been started but +// before anything waits on it: on builds where ClosePseudoConsole blocks until +// the console host exits, the host stays alive as long as a surviving client +// does, and that client only goes away through the kill — so ordering the kill +// after the close would deadlock in exactly the case the kill exists for. +// +// The pipe ends go first so that the pseudoconsole close, which flushes the +// client's pending output into the output pipe, cannot wedge on a pipe nobody is +// draining. A Read blocked at this moment is still draining and keeps its handle +// alive until it returns, which it does as soon as the far end goes away. +func (c *conPTY) shutdown(kill func()) { + _ = c.in.Close() + _ = c.out.Close() + done := c.closeHPCON() + + if kill != nil { + kill() + } + c.reapConhost() + + select { + case <-done: + case <-time.After(hpconCloseTimeout): + // Left to finish on its own. It only touches the pseudoconsole's own + // handles from here; both pipe ends are already released above. + } +} + +// teardown releases the pseudoconsole on the start error paths, where no child +// exists yet and so there is nothing to kill. +func (c *conPTY) teardown() { c.shutdown(nil) } + +// conhostScanMu serializes CreatePseudoConsole and the process scans around it, +// so concurrent starts cannot confuse each other's "which conhost is new" diff. +var conhostScanMu sync.Mutex + +// conhostChildren returns the pids of the conhost.exe processes that are direct +// children of this process. Errors just yield a smaller set: identifying the +// console host is best effort and never fails a start. +func conhostChildren() map[uint32]bool { + pids := map[uint32]bool{} + snap, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPPROCESS, 0) + if err != nil { + return pids + } + defer func() { _ = windows.CloseHandle(snap) }() + + me := uint32(os.Getpid()) + var pe windows.ProcessEntry32 + pe.Size = uint32(unsafe.Sizeof(pe)) + for err := windows.Process32First(snap, &pe); err == nil; err = windows.Process32Next(snap, &pe) { + if pe.ParentProcessID == me && strings.EqualFold(windows.UTF16ToString(pe.ExeFile[:]), "conhost.exe") { + pids[pe.ProcessID] = true + } + } + return pids +} + +// openNewConhostChild returns a handle to the single conhost child that appeared +// since the before scan, or 0 if there is not exactly one candidate or it cannot +// be opened. +func openNewConhostChild(before map[uint32]bool) windows.Handle { + var found []uint32 + for pid := range conhostChildren() { + if !before[pid] { + found = append(found, pid) + } + } + if len(found) != 1 { + return 0 + } + h, err := windows.OpenProcess(windows.SYNCHRONIZE|windows.PROCESS_TERMINATE, false, found[0]) + if err != nil { + return 0 + } + return h +} diff --git a/internal/terminal/jobobject_windows.go b/internal/terminal/jobobject_windows.go new file mode 100644 index 0000000..b53f7cb --- /dev/null +++ b/internal/terminal/jobobject_windows.go @@ -0,0 +1,92 @@ +//go:build windows + +package terminal + +import ( + "fmt" + "sync" + "unsafe" + + "golang.org/x/sys/windows" +) + +// jobObject holds the ConPTY child and every process it goes on to spawn, so +// the session can terminate the whole tree at once. Closing the pseudoconsole +// only delivers CTRL_CLOSE_EVENT to the clients attached at that moment, which +// misses a child still starting up and any grandchild spawned while the console +// is going down, and clients are free to ignore it — the job kill is what +// actually reaps all of those. +// +// The handle is guarded because Kill (from the caller, or from the context +// watcher) and Close race for it: terminating through a handle that Close has +// already freed would either fail or, once the handle value is reused, act on an +// unrelated object. +type jobObject struct { + mu sync.Mutex + h windows.Handle + closed bool +} + +// newJobObject creates a job that kills its members when the last handle to it +// goes away. That doubles as a safety net: if the process exits without running +// a teardown, the handle is closed for us and the tree is still reaped. +func newJobObject() (*jobObject, error) { + h, err := windows.CreateJobObject(nil, nil) + if err != nil { + return nil, fmt.Errorf("CreateJobObject: %w", err) + } + limits := windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION{ + BasicLimitInformation: windows.JOBOBJECT_BASIC_LIMIT_INFORMATION{ + LimitFlags: windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + }, + } + if _, err := windows.SetInformationJobObject( + h, windows.JobObjectExtendedLimitInformation, + uintptr(unsafe.Pointer(&limits)), uint32(unsafe.Sizeof(limits)), + ); err != nil { + _ = windows.CloseHandle(h) + return nil, fmt.Errorf("SetInformationJobObject: %w", err) + } + return &jobObject{h: h}, nil +} + +// assign puts a process into the job. The child is created suspended so that +// this lands before it runs its first instruction, which is what guarantees +// every descendant it ever spawns is in the job from the start. +func (j *jobObject) assign(process windows.Handle) error { + j.mu.Lock() + defer j.mu.Unlock() + if j.closed { + return fmt.Errorf("AssignProcessToJobObject: job already closed") + } + if err := windows.AssignProcessToJobObject(j.h, process); err != nil { + return fmt.Errorf("AssignProcessToJobObject: %w", err) + } + return nil +} + +// terminate kills every process still in the job. It is safe to call when the +// tree is already gone (an empty job terminates successfully) and after close, +// which reports nothing left to do rather than touching a freed handle. +func (j *jobObject) terminate() error { + j.mu.Lock() + defer j.mu.Unlock() + if j.closed { + return nil + } + if err := windows.TerminateJobObject(j.h, 1); err != nil { + return fmt.Errorf("TerminateJobObject: %w", err) + } + return nil +} + +// close releases the job handle, which also kills anything still in it. +func (j *jobObject) close() error { + j.mu.Lock() + defer j.mu.Unlock() + if j.closed { + return nil + } + j.closed = true + return windows.CloseHandle(j.h) +} diff --git a/internal/terminal/session.go b/internal/terminal/session.go index 0465d0d..962424f 100644 --- a/internal/terminal/session.go +++ b/internal/terminal/session.go @@ -1,8 +1,9 @@ // Package terminal abstracts launching a child process attached to a // pseudo-terminal, so provider triggers can drive an interactive CLI (Claude // Code, Codex) without depending on a specific PTY backend. Unix/macOS uses a -// real PTY (github.com/creack/pty); Windows is not supported yet (a ConPTY -// backend is tracked separately) and returns an error from Start. +// real PTY (github.com/creack/pty); Windows uses a ConPTY pseudoconsole, with +// the child and its descendants held in a job object so the whole tree can be +// torn down. package terminal import ( @@ -31,7 +32,8 @@ type Session interface { // Start launches name (with args) attached to a new pseudo-terminal and returns // a Session that owns the child's lifecycle. Cancelling ctx terminates the -// child. On platforms without PTY support it returns an error and a nil Session. +// child. It returns an error and a nil Session if the terminal or the child +// cannot be created. func Start(ctx context.Context, name string, args []string) (Session, error) { return start(ctx, name, args) } diff --git a/internal/terminal/session_windows.go b/internal/terminal/session_windows.go index 5786a88..d9c8d81 100644 --- a/internal/terminal/session_windows.go +++ b/internal/terminal/session_windows.go @@ -4,14 +4,230 @@ package terminal import ( "context" - "errors" + "fmt" + "os/exec" + "sync" + "unsafe" + + "golang.org/x/sys/windows" ) -// ErrUnsupported is returned by Start on Windows until the ConPTY backend lands. -// The interactive provider triggers require a real pseudo-terminal to anchor the -// subscription window, which github.com/creack/pty does not provide on Windows. -var ErrUnsupported = errors.New("interactive PTY sessions are not supported on Windows yet") +// windowsSession runs the child under a ConPTY pseudoconsole. Read and Write +// are the parent ends of the pipes wired to that console, and the child, plus +// anything it spawns, lives in a job object so the whole tree can be reaped. +type windowsSession struct { + pty *conPTY + job *jobObject + + // processDone is closed once the child has exited and waitErr holds its + // result. The real wait runs exactly once, in a background waiter started by + // start, so the process handle is released even if nobody ever calls Wait. + processDone chan struct{} + waitErr error + + closeOnce sync.Once +} + +func start(ctx context.Context, name string, args []string) (Session, error) { + // exec.Command is used only to resolve the executable and assemble argv the + // way os/exec would; the process itself has to be created by hand, because + // attaching a pseudoconsole needs an extended startup info block that + // (*exec.Cmd).Start does not expose. + cmd := exec.Command(name, args...) + if cmd.Err != nil { + return nil, cmd.Err + } + + pty, err := newConPTY(conPTYSize) + if err != nil { + return nil, err + } + job, err := newJobObject() + if err != nil { + pty.teardown() + return nil, err + } + proc, err := startAttached(cmd, pty.hpc, job) + if err != nil { + // Closing the job kills anything that did get assigned to it. + _ = job.close() + pty.teardown() + return nil, err + } + + s := &windowsSession{pty: pty, job: job, processDone: make(chan struct{})} + go s.wait(proc) + go s.watch(ctx) + return s, nil +} + +// startAttached creates the child attached to the pseudoconsole and returns its +// process handle, which the caller's waiter owns from then on. +// +// The startup info is an extended block whose Cb covers the whole STARTUPINFOEX +// (not just the STARTUPINFO inside it), carrying the pseudoconsole attribute, +// with EXTENDED_STARTUPINFO_PRESENT set so the attribute list is read at all. +// +// It also declares STARTF_USESTDHANDLES with all three handles left NULL, which +// is what actually attaches the child's stdio to the pseudoconsole rather than +// to ours. Without the flag, CreateProcess duplicates this process's standard +// handles into the child (it only skips that for CREATE_NEW_CONSOLE, +// CREATE_NO_WINDOW and DETACHED_PROCESS), and the console attach path replaces +// a standard handle "if the existing value is NULL, or ... looks like a console +// pseudohandle" — so inherited *console* handles get re-pointed at the +// pseudoconsole, while inherited pipes and files survive and the child renders +// into them instead. Every way this package is actually used has non-console +// standard handles: `bg`/`watch` run detached with their output on a log file, +// and the tests run under `go test`, which collects output through a pipe. The +// symptom is stark — the pseudoconsole emits its opening handshake and nothing +// more, while the child's output turns up on the parent's stdout. +// +// NULL handles rather than the pipe ends are what the console documentation +// asks for here (a console process started with no standard handles has them +// "filled automatically with appropriate handles to a new console", which for +// this child is the pseudoconsole), and it is what the Windows console +// maintainers recommend for a ConPTY parent whose own output is redirected. It +// is also why handle inheritance stays off: nothing is being handed down, and +// the pseudoconsole duplicates the pipe ends it needs by itself. +func startAttached(cmd *exec.Cmd, hpc windows.Handle, job *jobObject) (windows.Handle, error) { + attrs, err := windows.NewProcThreadAttributeList(1) + if err != nil { + return 0, fmt.Errorf("NewProcThreadAttributeList: %w", err) + } + defer attrs.Delete() + + // The attribute value is the HPCON itself, not a pointer to it — an HPCON is + // already a pointer-sized handle. Spelling that as unsafe.Pointer(hpc) would + // trip go vet's unsafeptr check (a uintptr-based type converted straight to + // unsafe.Pointer), so reinterpret the handle's bits through its address: + // &hpc is a real pointer, so none of these conversions is the flagged cast, + // and the resulting value is identical. + if err := attrs.Update( + windows.PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, + *(*unsafe.Pointer)(unsafe.Pointer(&hpc)), + unsafe.Sizeof(hpc), + ); err != nil { + return 0, fmt.Errorf("UpdateProcThreadAttribute: %w", err) + } + + var si windows.StartupInfoEx + si.Cb = uint32(unsafe.Sizeof(si)) + si.ProcThreadAttributeList = attrs.List() + si.Flags |= windows.STARTF_USESTDHANDLES + si.StdInput, si.StdOutput, si.StdErr = 0, 0, 0 + + appName, err := windows.UTF16PtrFromString(cmd.Path) + if err != nil { + return 0, err + } + // ComposeCommandLine applies the quoting rules the Windows CRT parses back, + // so an argument containing spaces or quotes survives the round trip. + cmdLine, err := windows.UTF16PtrFromString(windows.ComposeCommandLine(cmd.Args)) + if err != nil { + return 0, err + } + + // A nil environment block inherits this process's environment, which is what + // the providers need (PATH, the CLI's own credentials in the user profile). + // CREATE_UNICODE_ENVIRONMENT documents that inherited block as Unicode. + var pi windows.ProcessInformation + if err := windows.CreateProcess( + appName, + cmdLine, + nil, // process security attributes + nil, // thread security attributes + false, // no handle inheritance; the pseudoconsole dups what it needs + windows.EXTENDED_STARTUPINFO_PRESENT|windows.CREATE_UNICODE_ENVIRONMENT|windows.CREATE_SUSPENDED, + nil, // environment: inherited + nil, // working directory: inherited + &si.StartupInfo, + &pi, + ); err != nil { + return 0, fmt.Errorf("CreateProcess: %w", err) + } + + // Created suspended so the job assignment lands before the child runs its + // first instruction; only then is every descendant it spawns in the job too. + if err := job.assign(pi.Process); err != nil { + _ = windows.TerminateProcess(pi.Process, 1) + _ = windows.CloseHandle(pi.Thread) + _ = windows.CloseHandle(pi.Process) + return 0, err + } + if _, err := windows.ResumeThread(pi.Thread); err != nil { + _ = windows.TerminateProcess(pi.Process, 1) + _ = windows.CloseHandle(pi.Thread) + _ = windows.CloseHandle(pi.Process) + return 0, fmt.Errorf("ResumeThread: %w", err) + } + _ = windows.CloseHandle(pi.Thread) + return pi.Process, nil +} + +// wait is the single waiter for the child. It records the exit status, starts +// the pseudoconsole close so a blocked Read ends up at EOF, and only then +// publishes the result — the close is started rather than awaited because +// ClosePseudoConsole may block long past the child's exit (see closeHPCON). +func (s *windowsSession) wait(proc windows.Handle) { + defer close(s.processDone) + s.waitErr = waitProcess(proc) + s.pty.closeHPCON() +} + +// waitProcess blocks until the process exits and maps its status onto +// (*exec.Cmd).Wait semantics: nil for a clean exit, an error otherwise. It owns +// the handle and releases it before returning. +func waitProcess(proc windows.Handle) error { + defer func() { _ = windows.CloseHandle(proc) }() + + event, err := windows.WaitForSingleObject(proc, windows.INFINITE) + if err != nil { + return fmt.Errorf("WaitForSingleObject: %w", err) + } + if event != windows.WAIT_OBJECT_0 { + return fmt.Errorf("waiting for child returned unexpected state %#x", event) + } + var code uint32 + if err := windows.GetExitCodeProcess(proc, &code); err != nil { + return fmt.Errorf("GetExitCodeProcess: %w", err) + } + if code != 0 { + return fmt.Errorf("exit status %d", code) + } + return nil +} + +// watch binds ctx cancellation to the child, mirroring what exec.CommandContext +// does on Unix. It ends with the child, so it cannot outlive the session. +func (s *windowsSession) watch(ctx context.Context) { + select { + case <-ctx.Done(): + _ = s.Kill() + case <-s.processDone: + } +} + +func (s *windowsSession) Read(p []byte) (int, error) { return s.pty.out.Read(p) } +func (s *windowsSession) Write(p []byte) (int, error) { return s.pty.in.Write(p) } + +func (s *windowsSession) Wait() error { + <-s.processDone + return s.waitErr +} + +func (s *windowsSession) Kill() error { return s.job.terminate() } -func start(_ context.Context, _ string, _ []string) (Session, error) { - return nil, ErrUnsupported +// Close tears the session down, once, in the order conPTY.shutdown documents: +// the pipe ends go first, the pseudoconsole close runs in the background, and +// the process tree is killed without waiting for it. Killing the tree is what +// unblocks a Read parked on the output pipe, so Close is safe to call while the +// providers are still draining the session. +func (s *windowsSession) Close() error { + s.closeOnce.Do(func() { + s.pty.shutdown(func() { + _ = s.job.terminate() + _ = s.job.close() + }) + }) + return nil } diff --git a/internal/terminal/session_windows_test.go b/internal/terminal/session_windows_test.go index 3708683..3e2740b 100644 --- a/internal/terminal/session_windows_test.go +++ b/internal/terminal/session_windows_test.go @@ -5,43 +5,529 @@ package terminal import ( "context" "errors" + "io" + "os" + "path/filepath" + "strings" + "sync" "testing" "time" + "unsafe" + + "golang.org/x/sys/windows" +) + +// Every test here drives a real child process, so every wait is bounded: a +// regression in the teardown ordering shows up as a deadlock, and a deadlocked +// test must fail rather than hang until the package timeout. +const ( + waitTimeout = 15 * time.Second + outputTimeout = 15 * time.Second ) -// TestStartUnsupportedOnWindows verifies the current Windows behavior: Start -// reports ErrUnsupported, returns a nil Session, and does so promptly (no hang). -// The ConPTY backend will replace this stub; until then this pins the contract. -func TestStartUnsupportedOnWindows(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() +// cmdExe is the child used throughout. It is a console application that reads +// its input from the console (which is what makes the stdin round trip a real +// test of the pseudoconsole rather than of a pipe), and it is present on every +// Windows install. The real CLIs are deliberately never run from tests: they +// would spend the account's quota. +const cmdExe = "cmd.exe" - type result struct { - sess Session - err error +// interactiveArgs keeps a cmd.exe session alive reading from the console. /Q +// turns off command echo so the output is mostly what commands actually print, +// and /D skips any AutoRun command the machine may have configured. +var interactiveArgs = []string{"/Q", "/D", "/K"} + +func startSession(t *testing.T, ctx context.Context, name string, args ...string) Session { + t.Helper() + sess, err := Start(ctx, name, args) + if err != nil { + t.Fatalf("Start(%s %v) = %v", name, args, err) } - done := make(chan result, 1) + return sess +} + +// output collects everything a session renders, on its own goroutine, the way +// the providers drain a session with io.Copy. +type output struct { + mu sync.Mutex + buf []byte + err error + done chan struct{} +} + +func drain(sess Session) *output { + o := &output{done: make(chan struct{})} go func() { - // A harmless command line; it must never actually run on the stub path. - sess, err := Start(ctx, "cmd", []string{"/c", "echo hi"}) - done <- result{sess: sess, err: err} + defer close(o.done) + b := make([]byte, 4096) + for { + n, err := sess.Read(b) + o.mu.Lock() + o.buf = append(o.buf, b[:n]...) + if err != nil { + o.err = err + o.mu.Unlock() + return + } + o.mu.Unlock() + } }() + return o +} +func (o *output) String() string { + o.mu.Lock() + defer o.mu.Unlock() + return string(o.buf) +} + +// readErr is valid once the drain goroutine has finished. +func (o *output) readErr() error { + o.mu.Lock() + defer o.mu.Unlock() + return o.err +} + +// waitFor blocks until the session has rendered marker, or the timeout expires. +func (o *output) waitFor(marker string, timeout time.Duration) bool { + deadline := time.Now().Add(timeout) + for { + if strings.Contains(o.String(), marker) { + return true + } + if time.Now().After(deadline) { + return false + } + select { + case <-o.done: + // The stream ended; one last look at what it carried. + return strings.Contains(o.String(), marker) + case <-time.After(20 * time.Millisecond): + } + } +} + +// waitFinished waits for the drain goroutine to stop reading. +func (o *output) waitFinished(timeout time.Duration) bool { select { - case r := <-done: - if r.err == nil { - if r.sess != nil { - _ = r.sess.Close() + case <-o.done: + return true + case <-time.After(timeout): + return false + } +} + +// waitResult runs Wait on its own goroutine so a wedged teardown fails the test +// instead of hanging it. +func waitResult(sess Session, timeout time.Duration) (error, bool) { + done := make(chan error, 1) + go func() { done <- sess.Wait() }() + select { + case err := <-done: + return err, true + case <-time.After(timeout): + return nil, false + } +} + +func TestSessionCleanExit(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, "/c", "exit 0") + defer sess.Close() + + err, ok := waitResult(sess, waitTimeout) + if !ok { + t.Fatal("Wait did not return for a child that exits immediately") + } + if err != nil { + t.Fatalf("Wait = %v, want nil for a clean exit", err) + } +} + +func TestSessionNonZeroExit(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, "/c", "exit 3") + defer sess.Close() + + err, ok := waitResult(sess, waitTimeout) + if !ok { + t.Fatal("Wait did not return for a child that exits immediately") + } + if err == nil { + t.Fatal("Wait = nil, want an error for exit status 3") + } + if !strings.Contains(err.Error(), "3") { + t.Fatalf("Wait = %v, want the exit status in the message", err) + } +} + +func TestSessionRendersChildOutput(t *testing.T) { + const marker = "__LIMITPING_STDOUT_OK__" + sess := startSession(t, context.Background(), cmdExe, "/c", "echo "+marker) + defer sess.Close() + + out := drain(sess) + if !out.waitFor(marker, outputTimeout) { + t.Fatalf("child output never reached the pseudoconsole; got %q", out.String()) + } +} + +// TestSessionStdinRoundTrip is the test the whole backend exists for: the +// providers steer the interactive CLIs by writing keystrokes, so a write has to +// reach the child as console input and be acted on. +// +// The marker is assembled from an environment variable the child sets itself, +// because the console echoes typed input straight back to the output pipe: +// searching the output for a marker that was also typed would pass on the echo +// alone. `__LIMITPING_STDIN_%LP%__` is what gets echoed, and only running the +// command can turn it into `__LIMITPING_STDIN_OK__`. +func TestSessionStdinRoundTrip(t *testing.T) { + const marker = "__LIMITPING_STDIN_OK__" + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + defer sess.Close() + + out := drain(sess) + if _, err := sess.Write([]byte("set LP=OK\r")); err != nil { + t.Fatalf("Write(set) = %v", err) + } + if _, err := sess.Write([]byte("echo __LIMITPING_STDIN_%LP%__\r")); err != nil { + t.Fatalf("Write(echo) = %v", err) + } + if !out.waitFor(marker, outputTimeout) { + t.Fatalf("the child never ran the command written to its stdin; got %q", out.String()) + } + + // An explicit status keeps the exit unambiguous: bare `exit` carries + // whatever ERRORLEVEL the session happens to be holding. + if _, err := sess.Write([]byte("exit 0\r")); err != nil { + t.Fatalf("Write(exit) = %v", err) + } + err, ok := waitResult(sess, waitTimeout) + if !ok { + t.Fatal("Wait did not return after the child was told to exit") + } + if err != nil { + t.Fatalf("Wait = %v, want nil after `exit 0`", err) + } +} + +// TestSessionLaunchesCmdShim covers the shape an npm-installed CLI takes on +// Windows: a .cmd shim rather than a native executable. CreateProcess runs those +// through cmd.exe itself, and the arguments have to survive the round trip +// through the composed command line, quoting and all. +func TestSessionLaunchesCmdShim(t *testing.T) { + shim := filepath.Join(t.TempDir(), "limitping-probe.cmd") + script := "@echo off\r\necho __LIMITPING_SHIM__ [%~1] [%~2]\r\n" + if err := os.WriteFile(shim, []byte(script), 0o600); err != nil { + t.Fatalf("writing the shim: %v", err) + } + + sess := startSession(t, context.Background(), shim, "hello world", "plain") + defer sess.Close() + + out := drain(sess) + if !out.waitFor("__LIMITPING_SHIM__ [hello world] [plain]", outputTimeout) { + t.Fatalf("the shim did not run with its arguments intact; got %q", out.String()) + } + err, ok := waitResult(sess, waitTimeout) + if !ok { + t.Fatal("Wait did not return after the shim finished") + } + if err != nil { + t.Fatalf("Wait = %v, want nil", err) + } +} + +func TestSessionKill(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + defer sess.Close() + + if err := sess.Kill(); err != nil { + t.Fatalf("Kill = %v", err) + } + if _, ok := waitResult(sess, waitTimeout); !ok { + t.Fatal("Wait did not return after Kill") + } + // Killing a child that is already gone must stay safe. + if err := sess.Kill(); err != nil { + t.Fatalf("second Kill = %v", err) + } +} + +func TestSessionContextCancel(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + sess := startSession(t, ctx, cmdExe, interactiveArgs...) + defer sess.Close() + + cancel() + if _, ok := waitResult(sess, waitTimeout); !ok { + t.Fatal("Wait did not return after the context was cancelled") + } +} + +func TestSessionCloseIsIdempotent(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + + if err := sess.Close(); err != nil { + t.Fatalf("first Close = %v", err) + } + if err := sess.Close(); err != nil { + t.Fatalf("second Close = %v", err) + } + if _, ok := waitResult(sess, waitTimeout); !ok { + t.Fatal("Wait did not return after Close") + } +} + +// TestSessionReadEOFsOnChildExit pins the behavior the Unix master fd gives for +// free: once the child is gone the reader must reach end of input. ConPTY keeps +// the output pipe alive until the pseudoconsole is closed, so this only holds +// because the waiter closes it on child exit. +func TestSessionReadEOFsOnChildExit(t *testing.T) { + const marker = "__LIMITPING_EOF_OK__" + sess := startSession(t, context.Background(), cmdExe, "/c", "echo "+marker) + defer sess.Close() + + out := drain(sess) + if !out.waitFinished(outputTimeout) { + t.Fatalf("Read never returned after the child exited; got %q", out.String()) + } + if err := out.readErr(); !errors.Is(err, io.EOF) { + t.Fatalf("Read error = %v, want io.EOF", err) + } + if !strings.Contains(out.String(), marker) { + t.Fatalf("output ended without the child's output; got %q", out.String()) + } +} + +// TestSessionCloseUnblocksRead covers the shape the providers use: a goroutine +// parked in Read while the session is closed from elsewhere. Both it and Wait +// have to come back, which is what the teardown ordering in conPTY.shutdown is +// for — a Close that waited on ClosePseudoConsole before killing the tree would +// hang here. +func TestSessionCloseUnblocksRead(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + out := drain(sess) + + // Let the shell get far enough to be sitting on the console read, so the + // close lands on a genuinely blocked Read rather than on the startup burst. + if !out.waitFor(">", outputTimeout) { + t.Logf("no prompt seen before Close; got %q", out.String()) + } + + if err := sess.Close(); err != nil { + t.Fatalf("Close = %v", err) + } + if !out.waitFinished(waitTimeout) { + t.Fatal("Read stayed blocked after Close") + } + if _, ok := waitResult(sess, waitTimeout); !ok { + t.Fatal("Wait did not return after Close") + } +} + +// TestSessionRapidStartStop shakes out handle and conhost leaks, and the races +// between a start and the teardown of the session before it. +func TestSessionRapidStartStop(t *testing.T) { + for i := 0; i < 20; i++ { + sess := startSession(t, context.Background(), cmdExe, "/c", "exit 0") + err, ok := waitResult(sess, waitTimeout) + if !ok { + t.Fatalf("iteration %d: Wait did not return", i) + } + if err != nil { + t.Fatalf("iteration %d: Wait = %v", i, err) + } + if err := sess.Close(); err != nil { + t.Fatalf("iteration %d: Close = %v", i, err) + } + } +} + +// TestSessionConcurrent runs several sessions at once: the conhost identification +// diffs this process's children around CreatePseudoConsole, so overlapping starts +// must not confuse each other. +func TestSessionConcurrent(t *testing.T) { + const sessions = 6 + var wg sync.WaitGroup + for i := 0; i < sessions; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + marker := "__LIMITPING_CONCURRENT_" + string(rune('A'+i)) + "__" + sess, err := Start(context.Background(), cmdExe, []string{"/c", "echo " + marker}) + if err != nil { + t.Errorf("session %d: Start = %v", i, err) + return + } + defer sess.Close() + + out := drain(sess) + if !out.waitFor(marker, outputTimeout) { + t.Errorf("session %d: never rendered its marker; got %q", i, out.String()) + } + if err, ok := waitResult(sess, waitTimeout); !ok { + t.Errorf("session %d: Wait did not return", i) + } else if err != nil { + t.Errorf("session %d: Wait = %v", i, err) } - t.Fatal("Start returned a nil error on Windows; expected unsupported") + }(i) + } + wg.Wait() +} + +// jobBasicAccounting mirrors JOBOBJECT_BASIC_ACCOUNTING_INFORMATION, which +// x/sys/windows does not declare. Only ActiveProcesses is read here, to observe +// the whole process tree rather than just the child this package holds a handle +// to. +type jobBasicAccounting struct { + TotalUserTime int64 + TotalKernelTime int64 + ThisPeriodTotalUserTime int64 + ThisPeriodTotalKernelTime int64 + TotalPageFaultCount uint32 + TotalProcesses uint32 + ActiveProcesses uint32 + TotalTerminatedProcesses uint32 +} + +func activeProcesses(t *testing.T, job *jobObject) uint32 { + t.Helper() + var info jobBasicAccounting + if err := windows.QueryInformationJobObject( + job.h, windows.JobObjectBasicAccountingInformation, + uintptr(unsafe.Pointer(&info)), uint32(unsafe.Sizeof(info)), nil, + ); err != nil { + t.Fatalf("QueryInformationJobObject = %v", err) + } + return info.ActiveProcesses +} + +// TestSessionKillsProcessTree checks that a grandchild spawned by the shell dies +// with the session. That is the point of creating the child suspended and +// assigning it to the job before it runs: closing the pseudoconsole only +// notifies the clients attached at that moment, so nothing but the job kill +// reaps a process that was started later. +func TestSessionKillsProcessTree(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + defer sess.Close() + + ws := sess.(*windowsSession) + out := drain(sess) + + // `start /b` runs the grandchild in the same console, where `pause` blocks + // on the console read and keeps it alive. + if _, err := sess.Write([]byte("start \"\" /b cmd.exe /Q /D /c pause\r")); err != nil { + t.Fatalf("Write = %v", err) + } + + deadline := time.Now().Add(outputTimeout) + for activeProcesses(t, ws.job) < 2 { + if time.Now().After(deadline) { + t.Fatalf("the grandchild never joined the job; output %q", out.String()) + } + time.Sleep(20 * time.Millisecond) + } + + if err := sess.Kill(); err != nil { + t.Fatalf("Kill = %v", err) + } + deadline = time.Now().Add(waitTimeout) + for { + if n := activeProcesses(t, ws.job); n == 0 { + break + } else if time.Now().After(deadline) { + t.Fatalf("%d processes still alive in the job after Kill", n) + } + time.Sleep(20 * time.Millisecond) + } +} + +// TestConhostReturnsToBaseline checks the console hosts are reaped rather than +// accumulating: each pseudoconsole spawns one as a direct child of this process, +// and pre-24H2 builds can leave it running after its clients are killed. +func TestConhostReturnsToBaseline(t *testing.T) { + baseline := len(conhostChildren()) + + for i := 0; i < 4; i++ { + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + out := drain(sess) + out.waitFor(">", outputTimeout) + if err := sess.Close(); err != nil { + t.Fatalf("iteration %d: Close = %v", i, err) } - if !errors.Is(r.err, ErrUnsupported) { - t.Fatalf("Start error = %v, want ErrUnsupported", r.err) + if _, ok := waitResult(sess, waitTimeout); !ok { + t.Fatalf("iteration %d: Wait did not return", i) } - if r.sess != nil { - t.Fatalf("Start returned a non-nil Session on Windows: %v", r.sess) + } + + // The reap in Close is bounded but asynchronous on the OS side, so give the + // hosts a moment to disappear before concluding they leaked. + deadline := time.Now().Add(waitTimeout) + for { + n := len(conhostChildren()) + if n <= baseline { + return + } + if time.Now().After(deadline) { + t.Fatalf("conhost children = %d, baseline %d: console hosts leaked", n, baseline) } - case <-time.After(2 * time.Second): - t.Fatal("Start did not return promptly on Windows (possible hang)") + time.Sleep(50 * time.Millisecond) + } +} + +// TestSessionCtrlC records what a raw ETX byte does to a console child, because +// the Codex trigger stops its TUI by writing one. The pseudoconsole turns the +// byte into a real console interrupt: the shell abandons the line it was +// editing, so the command typed before it never runs. +// +// The interrupt has to arrive as its own write. Sent in the same burst as the +// line it should cancel, it is consumed as part of that line and the command +// runs anyway — which is how the providers use it (a lone write, repeated on a +// ticker), so this test writes it the same way. +// +// Two limits of the mechanism, both found here and neither pinned as a test: +// this conhost cancels the line without echoing the usual "^C", and a command +// already running (rather than reading the console) keeps running — the ETX is +// only turned into an interrupt when a client reads it, so ConPTY input alone +// does not stand in for GenerateConsoleCtrlEvent. Killing the job is what the +// providers fall back to, and that is covered by TestSessionKill. +func TestSessionCtrlC(t *testing.T) { + sess := startSession(t, context.Background(), cmdExe, interactiveArgs...) + defer sess.Close() + + out := drain(sess) + if _, err := sess.Write([]byte("set LP=RAN\r")); err != nil { + t.Fatalf("Write(set) = %v", err) + } + if !out.waitFor(">", outputTimeout) { + t.Fatalf("no prompt to type at; got %q", out.String()) + } + + // A line left unsubmitted. Waiting for the echo confirms the line editor has + // taken it, so the interrupt that follows cannot be swallowed as part of it. + if _, err := sess.Write([]byte("echo __LIMITPING_CTRLC_%LP%__")); err != nil { + t.Fatalf("Write(line) = %v", err) + } + if !out.waitFor("__LIMITPING_CTRLC_%LP%__", outputTimeout) { + t.Fatalf("the line was never echoed back; got %q", out.String()) + } + + if _, err := sess.Write([]byte{0x03}); err != nil { + t.Fatalf("Write(ETX) = %v", err) + } + // The Enter that would have run the line, had the interrupt not thrown it + // away, followed by a command that must run — it both proves the session + // survived the interrupt and orders the check below: once its marker is on + // screen, the interrupted line has had its chance. + if _, err := sess.Write([]byte("\r")); err != nil { + t.Fatalf("Write(CR) = %v", err) + } + if _, err := sess.Write([]byte("echo __LIMITPING_AFTER_%LP%__\r")); err != nil { + t.Fatalf("Write(after) = %v", err) + } + if !out.waitFor("__LIMITPING_AFTER_RAN__", outputTimeout) { + t.Fatalf("the session did not survive the interrupt; got %q", out.String()) + } + if strings.Contains(out.String(), "__LIMITPING_CTRLC_RAN__") { + t.Fatalf("the interrupted line ran anyway; got %q", out.String()) } } From 3b15087f034f91b13ebe397f45e59316a8a5a6c0 Mon Sep 17 00:00:00 2001 From: kjh0718 Date: Tue, 8 Sep 2026 00:56:45 +0900 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20Windows=20batch=20shim=EC=9D=84=20CO?= =?UTF-8?q?MSPEC=20=EA=B2=BD=EC=9C=A0=EB=A1=9C=20=EC=8B=A4=ED=96=89=20(#1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows에서 .cmd/.bat shim을 CreateProcess의 lpApplicationName으로 직접 실행하던 경로를 문서화된 COMSPEC 경유 방식으로 변경한다. 기존 방식은 일부 Windows 환경에서 동작하지만 cmd.exe의 추가 parsing을 고려하지 않아 공백이 있는 shim 경로가 실패하고, 특정 argument가 별도 명령으로 해석되는 command injection을 실제로 허용했다. - .cmd/.bat을 case-insensitive하게 판별하고 COMSPEC /d /v:off /s /c로 실행 - COMSPEC이 없으면 System32의 cmd.exe 절대 경로 사용 - native executable은 기존 ComposeCommandLine 경로 유지 - batch argument를 cmd 규칙에 맞게 quote하고 metacharacter injection 차단 - literal %, NUL, 개행은 batch를 통해 안전하게 보존할 수 없어 명시적으로 거부 - /v:off로 delayed expansion을 강제 비활성화해 !VAR!을 literal로 유지 - 오류 메시지에 사용자 argument 값을 직접 노출하지 않고 위치만 표시 - .BAT casing, 공백 경로, 공백 argument, metacharacter, injection, COMSPEC fallback, percent 거부 및 delayed expansion 테스트 추가 ConPTY, Job Object, Wait/Kill/Close 및 pipe lifecycle은 변경하지 않는다. Refs #1 --- internal/terminal/session_windows.go | 140 +++++++++- internal/terminal/session_windows_test.go | 323 +++++++++++++++++++++- 2 files changed, 451 insertions(+), 12 deletions(-) diff --git a/internal/terminal/session_windows.go b/internal/terminal/session_windows.go index d9c8d81..cdc2e17 100644 --- a/internal/terminal/session_windows.go +++ b/internal/terminal/session_windows.go @@ -4,8 +4,12 @@ package terminal import ( "context" + "errors" "fmt" + "os" "os/exec" + "path/filepath" + "strings" "sync" "unsafe" @@ -116,13 +120,15 @@ func startAttached(cmd *exec.Cmd, hpc windows.Handle, job *jobObject) (windows.H si.Flags |= windows.STARTF_USESTDHANDLES si.StdInput, si.StdOutput, si.StdErr = 0, 0, 0 - appName, err := windows.UTF16PtrFromString(cmd.Path) + app, line, err := prepareWindowsCommand(cmd) if err != nil { return 0, err } - // ComposeCommandLine applies the quoting rules the Windows CRT parses back, - // so an argument containing spaces or quotes survives the round trip. - cmdLine, err := windows.UTF16PtrFromString(windows.ComposeCommandLine(cmd.Args)) + appName, err := windows.UTF16PtrFromString(app) + if err != nil { + return 0, err + } + cmdLine, err := windows.UTF16PtrFromString(line) if err != nil { return 0, err } @@ -164,6 +170,132 @@ func startAttached(cmd *exec.Cmd, hpc windows.Handle, job *jobObject) (windows.H return pi.Process, nil } +// prepareWindowsCommand decides how CreateProcess should launch cmd: what to +// pass as lpApplicationName, and the command line to go with it. +// +// A native image is launched directly, with the arguments quoted the way the +// Windows CRT parses them back. A batch file is not an image, and CreateProcess +// documents the only supported way to run one: set lpApplicationName to the +// command interpreter and pass it /c plus the batch file. Some Windows builds do +// run a batch file handed straight to CreateProcess, which is why this went +// unnoticed, but nothing promises that and it is the wrong shape regardless — +// cmd.exe re-parses the command line under rules the CRT quoting does not +// account for, so an argument containing cmd metacharacters is not carried +// safely. Going through the interpreter explicitly makes the launch documented +// and lets the arguments be quoted for the parse that actually happens. +// +// This matters here because the npm-installed CLIs this package drives are .cmd +// shims on Windows, so the batch path is the common one, not the exotic one. +func prepareWindowsCommand(cmd *exec.Cmd) (appName, cmdLine string, err error) { + switch strings.ToLower(filepath.Ext(cmd.Path)) { + case ".cmd", ".bat": + return batchCommand(cmd.Path, cmd.Args[1:]) + default: + return cmd.Path, windows.ComposeCommandLine(cmd.Args), nil + } +} + +// batchCommand builds the interpreter invocation that runs a .cmd/.bat script. +// +// The command line is ` /d /v:off /s /c "