diff --git a/cmd/sftpshell/editor_boundaries_pty_test.go b/cmd/sftpshell/editor_boundaries_pty_test.go index 7b7eb65..ab2a8f6 100644 --- a/cmd/sftpshell/editor_boundaries_pty_test.go +++ b/cmd/sftpshell/editor_boundaries_pty_test.go @@ -5,7 +5,7 @@ package sftpshell import ( "context" "fmt" - "github.com/creack/pty" + pty "github.com/wentf9/xops-cli/internal/testpty" "io" "os" "strings" diff --git a/cmd/sftpshell/editor_history.go b/cmd/sftpshell/editor_history.go index 27615fb..2f95138 100644 --- a/cmd/sftpshell/editor_history.go +++ b/cmd/sftpshell/editor_history.go @@ -86,7 +86,7 @@ func (h *commandHistory) Append(line string) error { for _, pending := range h.pending { lines = appendHistoryLine(lines, pending) } - if err := writeCommandHistory(h.path, lines); err != nil { + if err := writeCommandHistory(ctx, h.path, lines); err != nil { return err } h.lines = lines @@ -144,7 +144,7 @@ func (h *commandHistory) withFileLock(ctx context.Context, operation func() erro defer func() { retErr = errors.Join(retErr, unlockHistory(lock)) }() return operation() } -func writeCommandHistory(path string, lines []string) (retErr error) { +func writeCommandHistory(ctx context.Context, path string, lines []string) (retErr error) { file, err := os.CreateTemp(filepath.Dir(path), ".xops-history-*") if err != nil { return fmt.Errorf("create history temporary file failed: %w", err) @@ -166,8 +166,33 @@ func writeCommandHistory(path string, lines []string) (retErr error) { return fmt.Errorf("close history failed: %w", err) } closed = true - if err = os.Rename(file.Name(), path); err != nil { + if err = renameCommandHistory(ctx, file.Name(), path); err != nil { return fmt.Errorf("replace history failed: %w", err) } return nil } + +// Windows metadata readers and scanners can briefly deny replacement even when +// cooperating writers hold the sidecar lock. Keep the temporary file and the +// writer lock until replacement succeeds or the existing history deadline ends. +func retryHistoryRename(parent context.Context, rename func() error, retryable func(error) bool) error { + ctx, cancel := context.WithTimeout(parent, 2*time.Second) + defer cancel() + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + var lastErr error + for { + if err := ctx.Err(); err != nil { + return errors.Join(lastErr, err) + } + lastErr = rename() + if lastErr == nil || !retryable(lastErr) { + return lastErr + } + select { + case <-ctx.Done(): + return errors.Join(lastErr, ctx.Err()) + case <-ticker.C: + } + } +} diff --git a/cmd/sftpshell/editor_history_replace_test.go b/cmd/sftpshell/editor_history_replace_test.go new file mode 100644 index 0000000..30649ea --- /dev/null +++ b/cmd/sftpshell/editor_history_replace_test.go @@ -0,0 +1,82 @@ +package sftpshell + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" +) + +func historyReplacementFixture(t *testing.T) (source, destination string) { + t.Helper() + dir := t.TempDir() + source, destination = filepath.Join(dir, "temporary"), filepath.Join(dir, "history") + if err := os.WriteFile(source, []byte("new history"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(destination, []byte("old history"), 0o600); err != nil { + t.Fatal(err) + } + return source, destination +} + +func assertHistoryFileContent(t *testing.T, path, want string) { + t.Helper() + got, err := os.ReadFile(path) + if err != nil || string(got) != want { + t.Fatalf("history content = %q, want %q, error: %v", got, want, err) + } +} + +func TestHistoryRenameRetriesTransientConflict(t *testing.T) { + source, destination := historyReplacementFixture(t) + transient := errors.New("transient file sharing conflict") + calls := 0 + err := retryHistoryRename(t.Context(), func() error { + calls++ + assertHistoryFileContent(t, destination, "old history") + if calls < 3 { + return transient + } + return os.Rename(source, destination) + }, func(err error) bool { return errors.Is(err, transient) }) + if err != nil || calls != 3 { + t.Fatalf("replacement calls=%d, error=%v", calls, err) + } + assertHistoryFileContent(t, destination, "new history") +} + +func TestHistoryRenameFailurePreservesFiles(t *testing.T) { + for _, scenario := range []string{"permanent error", "cancel during retry", "already cancelled"} { + t.Run(scenario, func(t *testing.T) { + source, destination := historyReplacementFixture(t) + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + if scenario == "already cancelled" { + cancel() + } + failure := errors.New("injected rename error") + calls := 0 + err := retryHistoryRename(ctx, func() error { + calls++ + if scenario == "cancel during retry" { + cancel() + } + return failure + }, func(error) bool { return scenario != "permanent error" }) + if scenario == "already cancelled" { + if calls != 0 || !errors.Is(err, context.Canceled) { + t.Fatalf("cancelled replacement: calls=%d, error=%v", calls, err) + } + } else if calls != 1 || !errors.Is(err, failure) { + t.Fatalf("failed replacement: calls=%d, error=%v", calls, err) + } + if scenario == "cancel during retry" && !errors.Is(err, context.Canceled) { + t.Fatalf("cancellation cause lost: %v", err) + } + assertHistoryFileContent(t, source, "new history") + assertHistoryFileContent(t, destination, "old history") + }) + } +} diff --git a/cmd/sftpshell/editor_history_replace_windows_test.go b/cmd/sftpshell/editor_history_replace_windows_test.go new file mode 100644 index 0000000..24b77bd --- /dev/null +++ b/cmd/sftpshell/editor_history_replace_windows_test.go @@ -0,0 +1,88 @@ +//go:build windows + +package sftpshell + +import ( + "context" + "os" + "sync" + "testing" + "time" + + "golang.org/x/sys/windows" +) + +func TestHistoryRenameWindowsErrorClassification(t *testing.T) { + for _, tc := range []struct { + err error + want bool + }{ + {windows.ERROR_ACCESS_DENIED, true}, + {&os.LinkError{Op: "rename", Err: windows.ERROR_SHARING_VIOLATION}, true}, + {windows.ERROR_FILE_NOT_FOUND, false}, + {windows.ERROR_PATH_NOT_FOUND, false}, + {windows.ERROR_DISK_FULL, false}, + {windows.ERROR_INVALID_PARAMETER, false}, + {context.DeadlineExceeded, false}, + } { + if got := isHistoryRenameRetryable(tc.err); got != tc.want { + t.Errorf("retry %v = %t, want %t", tc.err, got, tc.want) + } + } +} + +func TestHistoryRenameRetriesWindowsOpenReader(t *testing.T) { + source, destination := historyReplacementFixture(t) + path, err := windows.UTF16PtrFromString(destination) + if err != nil { + t.Fatal(err) + } + // Model a metadata reader or scanner that does not share delete access. + handle, err := windows.CreateFile(path, windows.GENERIC_READ, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL, 0) + if err != nil { + t.Fatal(err) + } + reader := os.NewFile(uintptr(handle), destination) + release := sync.OnceValue(reader.Close) + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + attempted := make(chan error, 1) + done := make(chan error, 1) + stopped := make(chan struct{}) + defer func() { + if err := release(); err != nil { + t.Error(err) + } + cancel() + <-stopped + }() + go func() { + defer close(stopped) + var first sync.Once + done <- retryHistoryRename(ctx, func() error { + err := os.Rename(source, destination) + first.Do(func() { attempted <- err }) + return err + }, isHistoryRenameRetryable) + }() + select { + case err := <-attempted: + if err == nil || !isHistoryRenameRetryable(err) { + t.Fatalf("open reader did not produce a sharing conflict: %v", err) + } + case <-ctx.Done(): + t.Fatal("history replacement did not start") + } + if err := release(); err != nil { + t.Fatal(err) + } + select { + case err := <-done: + if err != nil { + t.Fatalf("replacement failed after reader released: %v", err) + } + case <-ctx.Done(): + t.Fatal("history replacement did not finish") + } + assertHistoryFileContent(t, destination, "new history") +} diff --git a/cmd/sftpshell/editor_history_test.go b/cmd/sftpshell/editor_history_test.go index 5613dc5..ebfa6df 100644 --- a/cmd/sftpshell/editor_history_test.go +++ b/cmd/sftpshell/editor_history_test.go @@ -34,7 +34,7 @@ func TestCommandHistoryLimitsAndReload(t *testing.T) { if len(lines) != 500 || lines[0] != "200" { t.Fatal("history not bounded") } - if err := writeCommandHistory(path, lines); err != nil { + if err := writeCommandHistory(t.Context(), path, lines); err != nil { t.Fatal(err) } loaded, err = newCommandHistory(path) diff --git a/cmd/sftpshell/editor_history_unix.go b/cmd/sftpshell/editor_history_unix.go index 145eb71..669d97d 100644 --- a/cmd/sftpshell/editor_history_unix.go +++ b/cmd/sftpshell/editor_history_unix.go @@ -3,9 +3,11 @@ package sftpshell import ( + "context" "errors" - "golang.org/x/sys/unix" "os" + + "golang.org/x/sys/unix" ) func tryHistoryLock(file *os.File) (bool, error) { @@ -16,3 +18,7 @@ func tryHistoryLock(file *os.File) (bool, error) { return err == nil, err } func unlockHistory(file *os.File) error { return unix.Flock(int(file.Fd()), unix.LOCK_UN) } + +func renameCommandHistory(_ context.Context, source, destination string) error { + return os.Rename(source, destination) +} diff --git a/cmd/sftpshell/editor_history_windows.go b/cmd/sftpshell/editor_history_windows.go index 901d5a5..d6f5fd0 100644 --- a/cmd/sftpshell/editor_history_windows.go +++ b/cmd/sftpshell/editor_history_windows.go @@ -3,9 +3,11 @@ package sftpshell import ( + "context" "errors" - "golang.org/x/sys/windows" "os" + + "golang.org/x/sys/windows" ) func tryHistoryLock(file *os.File) (bool, error) { @@ -18,3 +20,11 @@ func tryHistoryLock(file *os.File) (bool, error) { func unlockHistory(file *os.File) error { return windows.UnlockFileEx(windows.Handle(file.Fd()), 0, 1, 0, &windows.Overlapped{}) } + +func renameCommandHistory(ctx context.Context, source, destination string) error { + return retryHistoryRename(ctx, func() error { return os.Rename(source, destination) }, isHistoryRenameRetryable) +} + +func isHistoryRenameRetryable(err error) bool { + return errors.Is(err, windows.ERROR_ACCESS_DENIED) || errors.Is(err, windows.ERROR_SHARING_VIOLATION) +} diff --git a/cmd/sftpshell/line_editor_pty_test.go b/cmd/sftpshell/line_editor_pty_test.go index 445be7e..a1932c4 100644 --- a/cmd/sftpshell/line_editor_pty_test.go +++ b/cmd/sftpshell/line_editor_pty_test.go @@ -15,8 +15,8 @@ import ( "testing" "time" - "github.com/creack/pty" "github.com/wentf9/xops-cli/internal/terminal" + pty "github.com/wentf9/xops-cli/internal/testpty" "github.com/wentf9/xops-cli/pkg/i18n" "github.com/wentf9/xops-cli/pkg/logger" "go.uber.org/goleak" diff --git a/docs/en/guide/sftp.md b/docs/en/guide/sftp.md index 9144c7b..62a8dde 100644 --- a/docs/en/guide/sftp.md +++ b/docs/en/guide/sftp.md @@ -68,6 +68,8 @@ Pasted newlines and tabs become spaces, and control characters are removed. Past If the history or lock file is unavailable at startup, a warning is displayed and the shell uses session-only history. Readable existing entries remain available, but this fallback does not write to disk. Normally, command history is stored in `~/.xops_sftp_history`, retaining up to 500 recent entries and ignoring empty input and consecutive duplicates. Confirmation answers and batch commands are not recorded. Confirmation prompts disable command completion and history browsing. History write failures are reported while retaining the command in the current session. Once writing recovers, pending entries are merged with disk history, retaining the most recent 500 entries. +On Windows, temporary file sharing conflicts are retried within the two-second history write deadline, retaining the original file and the lock shared by writers. Persistent conflicts and other errors still report a write failure without clearing existing history. + Before executing commands or entering a local/remote interactive program, the prompt stops reading and restores the terminal. The SFTP prompt resumes when the program finishes. Connection loss or cancellation ends pending input and completion tasks. External SIGINT or SIGTERM ends the session after pending prompt tasks stop and terminal state is restored. At an overwrite or removal confirmation, answering `n` skips only the current item; Ctrl+C stops the rest of the command and returns to the prompt. File operations already completed are not rolled back. diff --git a/docs/guide/sftp.md b/docs/guide/sftp.md index 6b3f92c..d4a0b4c 100644 --- a/docs/guide/sftp.md +++ b/docs/guide/sftp.md @@ -68,6 +68,8 @@ SFTP 提示符支持行编辑、命令历史以及本地和远端路径补全。 历史文件或锁文件在启动时不可用，会显示告警并降级为会话内历史，不影响 SFTP shell 启动；可读取的旧历史仍可浏览，此降级模式不会写入磁盘。正常情况下，命令历史保存到 `~/.xops_sftp_history`，最多保留最近 500 条，忽略空输入和连续重复命令。确认答案和批处理命令不写入历史；确认提示中不启用命令补全或历史浏览。历史写入失败会显示错误，会话内仍保留该命令；后续写入恢复时，会将待保存记录与磁盘历史合并写入，仍保留最近 500 条。 +Windows 上的短暂文件占用会在历史写入的两秒时限内重试，期间保留原文件和会话间写入锁。持续占用或其他错误仍会报告写入失败，不会清空原有历史。 + 执行命令和进入本地／远端交互程序前，提示符停止读取并恢复终端。程序退出后重新显示 SFTP 提示符；连接断开或取消会结束等待中的输入和补全任务。外部 SIGINT 或 SIGTERM 会终止会话，退出前等待提示符任务结束并恢复终端状态。 覆盖或删除确认中，回答 `n` 仅跳过当前项；Ctrl+C 则停止本条命令的剩余处理并返回提示符。已完成的文件操作不会撤销。 diff --git a/go.mod b/go.mod index d4f44ca..0ccdbd4 100644 --- a/go.mod +++ b/go.mod @@ -6,9 +6,9 @@ require ( charm.land/bubbles/v2 v2.2.1 charm.land/bubbletea/v2 v2.0.9 charm.land/huh/v2 v2.0.3 - charm.land/lipgloss/v2 v2.0.5 + charm.land/lipgloss/v2 v2.0.6 github.com/charmbracelet/colorprofile v0.4.3 - github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7 + github.com/charmbracelet/ultraviolet v0.0.0-20260811164956-006e29f97886 github.com/charmbracelet/x/ansi v0.11.8 github.com/charmbracelet/x/conpty v0.2.0 github.com/charmbracelet/x/term v0.2.2 @@ -51,7 +51,7 @@ require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/kr/fs v0.1.0 // indirect github.com/kr/text v0.2.0 // indirect - github.com/lucasb-eyer/go-colorful v1.4.0 // indirect + github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/mattn/go-runewidth v0.0.27 // indirect github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db // indirect github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect diff --git a/go.sum b/go.sum index 9ee9744..faa664b 100644 --- a/go.sum +++ b/go.sum @@ -4,8 +4,8 @@ charm.land/bubbletea/v2 v2.0.9 h1:DpJCMWKgzQK8SJv4zbKKFHAI10ymWy/evClPFk0k0f8= charm.land/bubbletea/v2 v2.0.9/go.mod h1:2SkdgoTXluXJHOUwAoRlRXF/28vklb1rFl6GcgV1/ss= charm.land/huh/v2 v2.0.3 h1:2cJsMqEPwSywGHvdlKsJyQKPtSJLVnFKyFbsYZTlLkU= charm.land/huh/v2 v2.0.3/go.mod h1:93eEveeeqn47MwiC3tf+2atZ2l7Is88rAtmZNZ8x9Wc= -charm.land/lipgloss/v2 v2.0.5 h1:kbNxgeeUOYv5J0YdpxFjfvf3dFvqH8Aci4zB6xqFtrY= -charm.land/lipgloss/v2 v2.0.5/go.mod h1:9oqhxt4yxIMe6q5A4kHr44DremZk7J9UNh74GlWa5nc= +charm.land/lipgloss/v2 v2.0.6 h1:EaGKeuA8FvF+v2BT5VmZd2LoYLaMZJXA5n34th8nCIQ= +charm.land/lipgloss/v2 v2.0.6/go.mod h1:ipDDJNSGa1hlwDtSfW1s2/xR8Vdhbut4PXh2zEKZd0Q= github.com/BurntSushi/toml v1.6.0 h1:dRaEfpa2VI55EwlIW72hMRHdWouJeRF7TPYhI+AUQjk= github.com/BurntSushi/toml v1.6.0/go.mod h1:ukJfTF/6rtPPRCnwkur4qwRxa8vTRFBF0uk2lLoLwho= github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= @@ -18,8 +18,8 @@ github.com/catppuccin/go v0.3.0 h1:d+0/YicIq+hSTo5oPuRi5kOpqkVA5tAsU6dNhvRu+aY= github.com/catppuccin/go v0.3.0/go.mod h1:8IHJuMGaUUjQM82qBrGNBv7LFq6JI3NnQCF6MOlZjpc= github.com/charmbracelet/colorprofile v0.4.3 h1:QPa1IWkYI+AOB+fE+mg/5/4HRMZcaXex9t5KX76i20Q= github.com/charmbracelet/colorprofile v0.4.3/go.mod h1:/zT4BhpD5aGFpqQQqw7a+VtHCzu+zrQtt1zhMt9mR4Q= -github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7 h1:3FmWoGNWK4STvqg0O0Aeav2T7rodWJAPeF0QpH+8gFw= -github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7/go.mod h1:f/jRa757WUmaOZrbPspXymbg/GnbF+rwe4OLsG7aXYo= +github.com/charmbracelet/ultraviolet v0.0.0-20260811164956-006e29f97886 h1:rdnVWKgJpTVXKuKuJyxDJ+NFJdUaUqGvyGy61OcvlbA= +github.com/charmbracelet/ultraviolet v0.0.0-20260811164956-006e29f97886/go.mod h1:nAw0d9PhFp1qdzi2xhQU5YOu5sVpDIHWlaW2Uz/bCro= github.com/charmbracelet/x/ansi v0.11.8 h1:JMFwp0CgDC2+jcOB162HH5k7I3FVbgFSMMYg7dSPBQQ= github.com/charmbracelet/x/ansi v0.11.8/go.mod h1:ZNN+3mXny/516oTQPLMPIBeSINvNJJQ8uQXDgbeJxY0= github.com/charmbracelet/x/conpty v0.2.0 h1:eKtA2hm34qNfgJCDp/M6Dc0gLy7e07YEK4qAdNGOvVY= @@ -80,8 +80,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= -github.com/lucasb-eyer/go-colorful v1.4.0 h1:UtrWVfLdarDgc44HcS7pYloGHJUjHV/4FwW4TvVgFr4= -github.com/lucasb-eyer/go-colorful v1.4.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= +github.com/lucasb-eyer/go-colorful v1.4.1 h1:1EO+WB73+EH8EVbzlrG3KLAfEypQWVHIBqlTf+2hNss= +github.com/lucasb-eyer/go-colorful v1.4.1/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-runewidth v0.0.27 h1:Feg/Oou5zI/wnpgDF6omIU0OokC9GxLC/WRknhVlIR0= github.com/mattn/go-runewidth v0.0.27/go.mod h1:3qAiGCV4Koz/yuveO58qUefmUTRm8r0IGEXZ9jeHp/8= github.com/mitchellh/colorstring v0.0.0-20190213212951-d06e56a500db h1:62I3jR2EmQ4l5rM/4FEfDWcRD+abF5XlKShorW5LRoQ= diff --git a/internal/testpty/pty_linux.go b/internal/testpty/pty_linux.go new file mode 100644 index 0000000..4f2329d --- /dev/null +++ b/internal/testpty/pty_linux.go @@ -0,0 +1,41 @@ +//go:build linux + +// Package testpty provides PTY pairs for terminal integration fixtures. +package testpty + +import ( + "errors" + "fmt" + "os" + "strconv" + + "golang.org/x/sys/unix" +) + +// Open returns a PTY pair. The caller owns both files. +// Pointer-aware ioctl helpers keep kernel output valid across Go stack moves. +func Open() (master, slave *os.File, retErr error) { + opened, err := os.OpenFile("/dev/ptmx", os.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + return nil, nil, fmt.Errorf("open PTY master: %w", err) + } + defer func() { + if retErr != nil { + if err := opened.Close(); err != nil { + retErr = errors.Join(retErr, fmt.Errorf("close PTY master after allocation failure: %w", err)) + } + } + }() + if err := unix.IoctlSetPointerInt(int(opened.Fd()), unix.TIOCSPTLCK, 0); err != nil { + return nil, nil, fmt.Errorf("unlock PTY slave: %w", err) + } + number, err := unix.IoctlGetInt(int(opened.Fd()), unix.TIOCGPTN) + if err != nil { + return nil, nil, fmt.Errorf("read PTY slave number: %w", err) + } + slave, err = os.OpenFile("/dev/pts/"+strconv.Itoa(number), os.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + return nil, nil, fmt.Errorf("open PTY slave: %w", err) + } + return opened, slave, nil +} diff --git a/internal/testpty/pty_linux_test.go b/internal/testpty/pty_linux_test.go new file mode 100644 index 0000000..d793362 --- /dev/null +++ b/internal/testpty/pty_linux_test.go @@ -0,0 +1,123 @@ +//go:build linux + +package testpty + +import ( + "context" + "errors" + "fmt" + "os" + "runtime" + "strconv" + "sync" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +func TestOpenPreservesPTYPairDuringStackRelocation(t *testing.T) { + // Occupy the first available PTY so an incorrectly zeroed slave number + // cannot accidentally identify the new master on an otherwise idle host. + master, slave, err := Open() + if err != nil { + t.Fatal(err) + } + defer func() { + if err := errors.Join(slave.Close(), master.Close()); err != nil { + t.Errorf("close reserved PTY pair: %v", err) + } + }() + ctx, cancel := context.WithTimeout(t.Context(), 30*time.Second) + gcDone := make(chan struct{}) + go func() { + defer close(gcDone) + for ctx.Err() == nil { + runtime.GC() + } + }() + var workers sync.WaitGroup + defer func() { cancel(); workers.Wait(); <-gcDone }() + for i := range 4096 { + done := make(chan error, 1) + workers.Go(func() { + growPTYStack(32) + if err := ctx.Err(); err != nil { + done <- err + return + } + done <- checkPTYAtDepth(i % 100) + }) + select { + case err := <-done: + if err != nil { + t.Fatalf("PTY pair %d: %v", i, err) + } + case <-ctx.Done(): + t.Fatal("PTY allocation did not finish within its deadline") + } + } +} + +// Grow before allocation so GC can shrink the stack during an ioctl. +// +//go:noinline +func growPTYStack(depth int) { + var padding [1024]byte + if depth > 0 { + growPTYStack(depth - 1) + } + runtime.KeepAlive(padding) +} + +//go:noinline +func checkPTYAtDepth(depth int) error { + var padding [127]byte + padding[0] = byte(depth) + var err error + if depth > 0 { + err = checkPTYAtDepth(depth - 1) + } else { + err = checkPTYPair() + } + runtime.KeepAlive(padding) + return err +} + +func checkPTYPair() (retErr error) { + master, slave, err := Open() + if err != nil { + return err + } + defer func() { + if err := errors.Join(slave.Close(), master.Close()); err != nil { + retErr = errors.Join(retErr, fmt.Errorf("close PTY pair: %w", err)) + } + }() + number, err := unix.IoctlGetInt(int(master.Fd()), unix.TIOCGPTN) + if err != nil { + return fmt.Errorf("read master PTY number: %w", err) + } + want, err := os.Stat("/dev/pts/" + strconv.Itoa(number)) + if err != nil { + return fmt.Errorf("stat master slave path: %w", err) + } + got, err := slave.Stat() + if err != nil { + return fmt.Errorf("stat opened slave: %w", err) + } + if !os.SameFile(got, want) { + return fmt.Errorf("opened %s instead of master's slave /dev/pts/%d", slave.Name(), number) + } + if err := Setsize(master, &Winsize{Rows: 31, Cols: 79, X: 632, Y: 496}); err != nil { + return err + } + size, err := unix.IoctlGetWinsize(int(slave.Fd()), unix.TIOCGWINSZ) + if err != nil { + return fmt.Errorf("read slave window size: %w", err) + } + if size.Row != 31 || size.Col != 79 || size.Xpixel != 632 || size.Ypixel != 496 { + return fmt.Errorf("window size corrupted during ioctl: %+v", size) + } + return nil +} diff --git a/internal/testpty/pty_other.go b/internal/testpty/pty_other.go new file mode 100644 index 0000000..b07f0a3 --- /dev/null +++ b/internal/testpty/pty_other.go @@ -0,0 +1,14 @@ +//go:build !linux && !windows + +package testpty + +import ( + "os" + + "github.com/creack/pty" +) + +// Open returns a PTY pair. The caller owns both files. +func Open() (*os.File, *os.File, error) { + return pty.Open() +} diff --git a/internal/testpty/winsize_unix.go b/internal/testpty/winsize_unix.go new file mode 100644 index 0000000..3516ac3 --- /dev/null +++ b/internal/testpty/winsize_unix.go @@ -0,0 +1,24 @@ +//go:build !windows + +package testpty + +import ( + "fmt" + "os" + + "github.com/creack/pty" + "golang.org/x/sys/unix" +) + +type Winsize = pty.Winsize + +// Setsize keeps the window-size buffer reachable through the ioctl call. +func Setsize(file *os.File, size *Winsize) error { + err := unix.IoctlSetWinsize(int(file.Fd()), unix.TIOCSWINSZ, &unix.Winsize{ + Row: size.Rows, Col: size.Cols, Xpixel: size.X, Ypixel: size.Y, + }) + if err != nil { + return fmt.Errorf("set PTY window size: %w", err) + } + return nil +} diff --git a/pkg/tui/form_view_linux_test.go b/pkg/tui/form_view_linux_test.go index 6ac3678..1cc7a86 100644 --- a/pkg/tui/form_view_linux_test.go +++ b/pkg/tui/form_view_linux_test.go @@ -13,8 +13,8 @@ import ( "time" "charm.land/huh/v2" - "github.com/creack/pty" "github.com/hinshun/vt10x" + pty "github.com/wentf9/xops-cli/internal/testpty" "github.com/wentf9/xops-cli/pkg/i18n" "go.uber.org/goleak" ) diff --git a/pkg/tui/program_linux_test.go b/pkg/tui/program_linux_test.go index cd9af89..07b70a9 100644 --- a/pkg/tui/program_linux_test.go +++ b/pkg/tui/program_linux_test.go @@ -6,6 +6,7 @@ import ( "context" "errors" "io" + "os" "strings" "sync" "testing" @@ -14,7 +15,7 @@ import ( tea "charm.land/bubbletea/v2" "charm.land/lipgloss/v2" "github.com/charmbracelet/x/term" - "github.com/creack/pty" + pty "github.com/wentf9/xops-cli/internal/testpty" "go.uber.org/goleak" "golang.org/x/sys/unix" ) @@ -62,10 +63,8 @@ func testProgramShutdown(t *testing.T, input string, cancelProgram bool, filter ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() output := &programReadyWriter{ready: make(chan struct{})} - done := make(chan error, 1) - go func() { - done <- Run(ctx, model, slave, output) - }() + done, stop := startProgramTest(ctx, model, slave, output) + defer stop() select { case <-output.ready: case <-ctx.Done(): @@ -113,19 +112,27 @@ func TestProgramUnlockReleasesAndRestartsInput(t *testing.T) { } model := newV2TestModel(t) restored := make(chan error, 1) + resumeUnlock := make(chan struct{}) + releaseUnlock := sync.OnceFunc(func() { close(resumeUnlock) }) model.vaultControl = func(ctx context.Context, unlock bool) error { state, err := unix.IoctlGetTermios(int(slave.Fd()), unix.TCGETS) if err == nil && (!unlock || *state != *before) { err = errors.New("unlock did not receive the restored terminal") } restored <- err - return err + select { + case <-resumeUnlock: + return err + case <-ctx.Done(): + return errors.Join(err, ctx.Err()) + } } ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) defer cancel() output := &programReadyWriter{ready: make(chan struct{})} - done := make(chan error, 1) - go func() { done <- Run(ctx, model, slave, output) }() + done, stop := startProgramTest(ctx, model, slave, output) + defer stop() + defer releaseUnlock() select { case <-output.ready: case <-ctx.Done(): @@ -142,6 +149,10 @@ func TestProgramUnlockReleasesAndRestartsInput(t *testing.T) { case <-ctx.Done(): t.Fatal("unlock did not run") } + releaseUnlock() + // The callback reports the restored terminal before returning. A Ctrl+C + // sent then is consumed by canonical ISIG handling, not the TUI decoder. + waitProgramRawInput(t, ctx, slave) if _, err := io.WriteString(master, "\x03"); err != nil { t.Fatal(err) } @@ -155,6 +166,38 @@ func TestProgramUnlockReleasesAndRestartsInput(t *testing.T) { } } +// Stop and join Run before closing its borrowed terminal, including on failures. +func startProgramTest(parent context.Context, model *Model, input *os.File, output io.Writer) (<-chan error, func()) { + ctx, cancel := context.WithCancel(parent) + done := make(chan error, 1) + stopped := make(chan struct{}) + go func() { + defer close(stopped) + done <- Run(ctx, model, input, output) + }() + return done, func() { cancel(); <-stopped } +} + +func waitProgramRawInput(t *testing.T, ctx context.Context, input *os.File) { + t.Helper() + ticker := time.NewTicker(time.Millisecond) + defer ticker.Stop() + for { + state, err := unix.IoctlGetTermios(int(input.Fd()), unix.TCGETS) + if err != nil { + t.Fatal(err) + } + if state.Lflag&(unix.ICANON|unix.ISIG) == 0 { + return + } + select { + case <-ticker.C: + case <-ctx.Done(): + t.Fatal("raw terminal input did not resume after unlock") + } + } +} + // Bubbles may finish an already-scheduled cursor blink after Run returns. // Allow that bounded timer, but never ignore a decoder or command goroutine. func assertProgramNoLeaks(t *testing.T, baseline goleak.Option) { @@ -187,8 +230,8 @@ func TestProgramRequestsAndDecodesBackground(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) defer cancel() output := &backgroundQueryWriter{queried: make(chan struct{})} - done := make(chan error, 1) - go func() { done <- Run(ctx, model, slave, output) }() + done, stop := startProgramTest(ctx, model, slave, output) + defer stop() select { case <-output.queried: case <-ctx.Done():